From 2e6bf94cb414f329d324c6bcef8740b8a510181c Mon Sep 17 00:00:00 2001 From: ArjanteMarvelde Date: Fri, 12 Aug 2022 11:15:30 +0200 Subject: [PATCH] V3.07 Q&D fix for I2C issue: i2c_write_blocking() returning prematurely, causing misses in register writes. Effect was erratic behaviour of the Si5351 clocks. General fix will follow. --- hmi.c | 208 ++++++++++++++++++++++++++---------------------------- monitor.c | 13 ++-- si5351.c | 159 +++++++++++++++++------------------------ si5351.h | 9 ++- uSDR.c | 4 +- 5 files changed, 179 insertions(+), 214 deletions(-) diff --git a/hmi.c b/hmi.c index eda5a46..27d3d38 100644 --- a/hmi.c +++ b/hmi.c @@ -147,20 +147,51 @@ bool ptt_active; // Resulting state #define MAX(x, y) ((x)>(y)?(x):(y)) // Get max value #endif + /* - * HMI State Machine, - * Handle event according to current state - * Code needs to be optimized + * GPIO IRQ callback routine + * Sets the detected event and invokes the HMI state machine */ -void hmi_handler(uint8_t event) +void hmi_callback(uint gpio, uint32_t events) { - /* Special case for TUNE state */ + uint8_t evt=HMI_E_NOEVENT; + + // Decide what the event was + switch (gpio) + { + case GP_ENC_A: // Encoder + if (events&GPIO_IRQ_EDGE_FALL) + evt = gpio_get(GP_ENC_B)?HMI_E_INCREMENT:HMI_E_DECREMENT; + break; + case GP_AUX_0: // Enter + if (events&GPIO_IRQ_EDGE_FALL) + evt = HMI_E_ENTER; + break; + case GP_AUX_1: // Escape + if (events&GPIO_IRQ_EDGE_FALL) + evt = HMI_E_ESCAPE; + break; + case GP_AUX_2: // Previous + if (events&GPIO_IRQ_EDGE_FALL) + evt = HMI_E_LEFT; + break; + case GP_AUX_3: // Next + if (events&GPIO_IRQ_EDGE_FALL) + evt = HMI_E_RIGHT; + break; + default: // Stray... + return; + } + + /** HMI State Machine **/ + + // Special case for TUNE state if (hmi_state == HMI_S_TUNE) { - switch (event) + switch (evt) { case HMI_E_ENTER: // Commit current value - si_setfreq(0, HMI_MULFREQ*(hmi_freq-FC_OFFSET)); // Commit frequency + // To be defined action break; case HMI_E_ESCAPE: // Enter submenus hmi_sub[hmi_state] = hmi_option; // Store selection (i.e. digit) @@ -185,8 +216,8 @@ void hmi_handler(uint8_t event) return; // Early bail-out } - /* Actions for other states */ - switch (event) + // Actions for other states + switch (evt) { case HMI_E_ENTER: hmi_sub[hmi_state] = hmi_option; // Store value for selected option @@ -213,102 +244,10 @@ void hmi_handler(uint8_t event) } } -/* - * GPIO IRQ callback routine - * Sets the detected event and invokes the HMI state machine - */ -void hmi_callback(uint gpio, uint32_t events) -{ - uint8_t evt=HMI_E_NOEVENT; - - switch (gpio) - { - case GP_ENC_A: // Encoder - if (events&GPIO_IRQ_EDGE_FALL) - evt = gpio_get(GP_ENC_B)?HMI_E_INCREMENT:HMI_E_DECREMENT; - break; - case GP_AUX_0: // Enter - if (events&GPIO_IRQ_EDGE_FALL) - evt = HMI_E_ENTER; - break; - case GP_AUX_1: // Escape - if (events&GPIO_IRQ_EDGE_FALL) - evt = HMI_E_ESCAPE; - break; - case GP_AUX_2: // Previous - if (events&GPIO_IRQ_EDGE_FALL) - evt = HMI_E_LEFT; - break; - case GP_AUX_3: // Next - if (events&GPIO_IRQ_EDGE_FALL) - evt = HMI_E_RIGHT; - break; - default: - return; - } - - hmi_handler(evt); // Invoke state machine -} /* - * Initialize the User interface - */ -void hmi_init(void) -{ - /* - * Notes on using GPIO interrupts: - * The callback handles interrupts for all GPIOs with IRQ enabled. - * Level interrupts don't seem to work properly. - * For debouncing, the GPIO pins should be pulled-up and connected to gnd with 100nF. - * PTT has separate debouncing logic - */ - - // Init input GPIOs - gpio_init_mask(GP_MASK_IN); - - // Enable pull-ups - gpio_pull_up(GP_ENC_A); - gpio_pull_up(GP_ENC_B); - gpio_pull_up(GP_AUX_0); - gpio_pull_up(GP_AUX_1); - gpio_pull_up(GP_AUX_2); - gpio_pull_up(GP_AUX_3); - gpio_pull_up(GP_PTT); - gpio_set_oeover(GP_PTT, GPIO_OVERRIDE_HIGH); // Enable output on PTT GPIO; bidirectional - - // Enable interrupt on level low - gpio_set_irq_enabled(GP_ENC_A, GPIO_IRQ_EDGE_ALL, true); - gpio_set_irq_enabled(GP_AUX_0, GPIO_IRQ_EDGE_ALL, true); - gpio_set_irq_enabled(GP_AUX_1, GPIO_IRQ_EDGE_ALL, true); - gpio_set_irq_enabled(GP_AUX_2, GPIO_IRQ_EDGE_ALL, true); - gpio_set_irq_enabled(GP_AUX_3, GPIO_IRQ_EDGE_ALL, true); - gpio_set_irq_enabled(GP_PTT, GPIO_IRQ_EDGE_ALL, false); - - // Set callback, one for all GPIO, not sure about correctness! - gpio_set_irq_enabled_with_callback(GP_ENC_A, GPIO_IRQ_EDGE_ALL, true, hmi_callback); - - // Initialize LCD and set VFO - hmi_state = HMI_S_TUNE; - hmi_option = 4; // Active kHz digit - hmi_freq = 7074000UL; // Initial frequency - - si_setfreq(0, HMI_MULFREQ*(hmi_freq-FC_OFFSET)); // Set freq to 7074 kHz (depends on mixer type) - si_setphase(0, 1); // Set phase to 90deg (depends on mixer type) - - ptt_state = PTT_DEBOUNCE; - ptt_active = false; - - dsp_setmode(hmi_sub[HMI_S_MODE]); - dsp_setvox(hmi_sub[HMI_S_VOX]); - dsp_setagc(hmi_sub[HMI_S_AGC]); - relay_setattn(hmi_pre[hmi_sub[HMI_S_PRE]]); - relay_setband(hmi_bpf[hmi_sub[HMI_S_BPF]]); - hmi_update = false; -} - -/* - * Redraw the display, representing current state - * This function is called regularly from the main loop. + * Redraw the 16x2 LCD display, representing current state + * This function is invoked regularly from the main loop. */ void hmi_evaluate(void) { @@ -377,9 +316,9 @@ void hmi_evaluate(void) /* Set parameters corresponding to latest entered option value */ - - // Frequency might have been changed in hmi_handler, so set anyway - si_setfreq(0, HMI_MULFREQ*(hmi_freq-FC_OFFSET)); + + // See if VFO needs update + si_evaluate(0, hmi_freq); // Check bandfilter setting (thanks Alex) if (hmi_freq < 2500000UL) band = REL_LPF2; @@ -407,3 +346,60 @@ void hmi_evaluate(void) } } + +/* + * Initialize the User interface + */ +void hmi_init(void) +{ + /* + * Notes on using GPIO interrupts: + * The callback handles interrupts for all GPIOs with IRQ enabled. + * Level interrupts don't seem to work properly. + * For debouncing, the GPIO pins should be pulled-up and connected to gnd with 100nF. + * PTT has separate debouncing logic + */ + + // Init input GPIOs + gpio_init_mask(GP_MASK_IN); + + // Enable pull-ups + gpio_pull_up(GP_ENC_A); + gpio_pull_up(GP_ENC_B); + gpio_pull_up(GP_AUX_0); + gpio_pull_up(GP_AUX_1); + gpio_pull_up(GP_AUX_2); + gpio_pull_up(GP_AUX_3); + gpio_pull_up(GP_PTT); + gpio_set_oeover(GP_PTT, GPIO_OVERRIDE_HIGH); // Enable output on PTT GPIO; bidirectional + + // Enable interrupt on level low + gpio_set_irq_enabled(GP_ENC_A, GPIO_IRQ_EDGE_ALL, true); + gpio_set_irq_enabled(GP_AUX_0, GPIO_IRQ_EDGE_ALL, true); + gpio_set_irq_enabled(GP_AUX_1, GPIO_IRQ_EDGE_ALL, true); + gpio_set_irq_enabled(GP_AUX_2, GPIO_IRQ_EDGE_ALL, true); + gpio_set_irq_enabled(GP_AUX_3, GPIO_IRQ_EDGE_ALL, true); + gpio_set_irq_enabled(GP_PTT, GPIO_IRQ_EDGE_ALL, false); + + // Set callback, one for all GPIO, not sure about correctness! + gpio_set_irq_enabled_with_callback(GP_ENC_A, GPIO_IRQ_EDGE_ALL, true, hmi_callback); + + // Initialize LCD and set VFO + hmi_state = HMI_S_TUNE; + hmi_option = 4; // Active kHz digit + hmi_freq = 7074000UL; // Initial frequency + + si_setphase(0, 1); // Set phase to 90deg (depends on mixer type) + si_evaluate(0, HMI_MULFREQ*(hmi_freq-FC_OFFSET)); // Set freq to 7074 kHz (depends on mixer type) + + ptt_state = PTT_DEBOUNCE; + ptt_active = false; + + dsp_setmode(hmi_sub[HMI_S_MODE]); + dsp_setvox(hmi_sub[HMI_S_VOX]); + dsp_setagc(hmi_sub[HMI_S_AGC]); + relay_setattn(hmi_pre[hmi_sub[HMI_S_PRE]]); + relay_setband(hmi_bpf[hmi_sub[HMI_S_BPF]]); + hmi_update = false; +} + diff --git a/monitor.c b/monitor.c index 5d552ac..f47445d 100644 --- a/monitor.c +++ b/monitor.c @@ -87,7 +87,7 @@ void mon_si(void) /* * Dumps the VFO registers */ -extern vfo_t vfo[2]; +vfo_t m_vfo; void mon_vfo(void) { int i; @@ -96,11 +96,12 @@ void mon_vfo(void) i = atoi(argv[1]); if ((i<0)||(i>1)) return; - printf("Frequency: %lu\n", vfo[i].freq); - printf("Phase : %u\n", (int)(vfo[i].phase)); - printf("Ri : %lu\n", (int)(vfo[i].ri)); - printf("MSi : %lu\n", (int)(vfo[i].msi)); - printf("MSN : %g\n\n", vfo[i].msn); + si_getvfo(i, &m_vfo); // Get local copy + printf("Frequency: %lu\n", m_vfo.freq); + printf("Phase : %u\n", (int)(m_vfo.phase)); + printf("Ri : %lu\n", (int)(m_vfo.ri)); + printf("MSi : %lu\n", (int)(m_vfo.msi)); + printf("MSN : %g\n\n", m_vfo.msn); } diff --git a/si5351.c b/si5351.c index ffd0063..40b39e3 100644 --- a/si5351.c +++ b/si5351.c @@ -189,30 +189,35 @@ Control Si5351 (see AN619): #define SI_VFO1CTL 0b00101101 // nonINT, PLLB, nonINV, SRC=MS, 4mA // PLL_RESET register 177 values -#define SI_PLLB_RST 0b10000000 // Reset PLL B -#define SI_PLLA_RST 0b00100000 // Reset PLL A +#define SI_PLLB_RST 0b10001100 // Reset PLL B +#define SI_PLLA_RST 0b00101100 // Reset PLL A #define SI_XTAL_FREQ 25001414UL // Replace with measured crystal frequency of XTAL for CL = 10pF (default) #define SI_MSN_LO ((0.4e9)/SI_XTAL_FREQ) // Should be 600M, but 400MHz works too #define SI_MSN_HI ((0.9e9)/SI_XTAL_FREQ) +#define SI_VCO_LO 400000000UL // Should be 600MHz, but 400MHz works too +#define SI_VCO_HI 900000000UL #define SI_PLL_C 1000000UL // Parameter c for PLL-A and -B setting vfo_t vfo[2]; // 0: clk0 / clk1 1: clk2 - -void si_setfreq(int i, uint32_t f) +int si_getvfo(int i, vfo_t *v) { - if ((i<0)||(i>1)) return; // Check VFO range - if (f>150000000) return; // Check frequency range - if (vfo[i].freq == f) return; // Anything to set at all? + if ((i<0)||(i>1)) return 0; // Check VFO range - vfo[i].freq = f; // Entry checks pass, so do the actual setting - vfo[i].flag |= 0x01; + v->freq = vfo[i].freq; + v->phase = vfo[i].phase; + v->ri = vfo[i].ri; + v->msi = vfo[i].msi; + v->msn = vfo[i].msn; + + return 1; } + void si_setphase(int i, uint8_t p) { if (i!=0) return; // Check VFO range @@ -220,7 +225,6 @@ void si_setphase(int i, uint8_t p) if (vfo[i].phase == p) return; // Anything to set at all? vfo[i].phase = p; // Entry checks pass, so do the actual setting - vfo[i].flag |= 0x02; } void si_enable(int i, bool en) @@ -309,7 +313,7 @@ void si_setmsn(int i) P3 = c */ -void si_setmsi(uint8_t i) +void si_setmsi(int i) { uint8_t data[16]; // I2C trx buffer uint32_t P1; @@ -317,7 +321,8 @@ void si_setmsi(uint8_t i) if ((i<0)||(i>1)) return; // Check VFO range - P1 = (uint32_t)(128*(uint32_t)floor(vfo[i].msi) - 512); + P1 = vfo[i].msi; // Upgrade msi to uint32_t + P1 = 128*P1-512; R = vfo[i].ri; R = (R&0xf0) ? ((R&0xc0)?((R&0x80)?7:6):(R&0x20)?5:4) : ((R&0x0c)?((R&0x08)?3:2):(R&0x02)?1:0); // quick log2(r) @@ -353,6 +358,7 @@ void si_setmsi(uint8_t i) data[1] = 0; // offset == 0 for 0deg i2c_write_blocking(i2c0, I2C_VFO, data, 2, false); } + sleep_us(500); if ((vfo[0].phase==PH180)||(vfo[0].phase==PH270)) // Phase is 180 or 270 deg? { data[0] = SI_CLK0_CTL; // set the invert flag @@ -367,112 +373,77 @@ void si_setmsi(uint8_t i) data[2] = SI_VFO0CTL; // CLK1: nonINV i2c_write_blocking(i2c0, I2C_VFO, data, 3, false); } + + // Reset PLL A (use with care, this causes a click) + sleep_us(500); + data[0] = SI_PLL_RESET; + data[1] = SI_PLLA_RST|SI_PLLB_RST; + i2c_write_blocking(i2c0, I2C_VFO, data, 2, false); } else { data[0] = SI_CLK2_CTL; // set the invert flag data[1] = SI_VFO1CTL; // CLK2: nonINV i2c_write_blocking(i2c0, I2C_VFO, data, 2, false); - } - - // Reset associated PLL - data[0] = SI_PLL_RESET; - data[1] = (i==1)?SI_PLLB_RST:SI_PLLA_RST; - i2c_write_blocking(i2c0, I2C_VFO, data, 2, false); + + // Reset PLL B (use with care, this causes a click) + sleep_us(500); + data[0] = SI_PLL_RESET; + data[1] = SI_PLLA_RST|SI_PLLB_RST; + i2c_write_blocking(i2c0, I2C_VFO, data, 2, false); + } } /* * This function needs to be invoked at regular intervals, e.g. 10x per sec. See hmi.c - * For each vfo, calculate required MSN setting, MSN = MSi*Ri*Fout/Fxtal + * For VFO i, calculate required MSN setting, MSN = MSi*Ri*Fout/Fxtal based on required frequency + * * If still in range, * then just set MSN registers * else, * recalculate MSi and Ri as well * set MSN, MSi and Ri registers (implicitly resets PLL) */ -void si_evaluate(void) +void si_evaluate(int i, uint32_t freq) { - double msn; + double msn; + uint32_t fvco; - if (vfo[0].flag) + if ((i<0)||(i>1)) return; // Check VFO range + if (vfo[i].freq == freq) return; // Nothing to do + + + fvco = freq*vfo[i].msi; // Required Fvco + if ((fvco>=SI_VCO_LO)&&(fvco=SI_MSN_LO)&&(msn3000000) - vfo[0].ri = 1; - else if (vfo[0].freq>1000000) - vfo[0].ri = 32; - else - vfo[0].ri = 128; - - // Set MSi - if ((vfo[0].freq >= 3000000)&&(vfo[0].freq < 6000000)) // Handle Low end of Ri=1 range - vfo[0].msi = (uint8_t)126; // Maximum MSi on Fvco=(4x126)MHz - else // Or calculate MSi on Fvco=750MHz - vfo[0].msi = (uint8_t)((750000000UL / (vfo[0].freq * vfo[0].ri)) & 0x000000fe); - - msn = (double)(vfo[0].msi); // Re-calculate MSN - msn = msn * (double)(vfo[0].ri); - msn = msn * (double)(vfo[0].freq) / SI_XTAL_FREQ; - vfo[0].msn = msn; - - vfo[0].phase = PH090; - - si_setmsn(0); - si_setmsi(0); - } - vfo[0].flag = 0; + vfo[i].msn = (double)fvco / SI_XTAL_FREQ; // Calculate required MSN + si_setmsn(i); // Set registers } - if (vfo[1].flag) + else { - msn = (double)(vfo[1].msi); // Re-calculate MSN - msn = msn * (double)(vfo[1].ri); - msn = msn * (double)(vfo[1].freq) / SI_XTAL_FREQ; + // Pre-scale Ri, stretch down Ri=1 range to 3MHz + // Otherwise use just 32 and 128 + vfo[i].ri = (freq<1000000UL)?128:((freq<3000000UL)?32 : 1); - if ((msn>=SI_MSN_LO)&&(msn3000000) - vfo[1].ri = 1; - else if (vfo[1].freq>1000000) - vfo[1].ri = 32; - else - vfo[1].ri = 128; - - // Set MSi - if ((vfo[1].freq >= 3000000)&&(vfo[1].freq < 6000000)) // Handle Low end of Ri=1 range - vfo[1].msi = (uint8_t)126; // Maximum MSi on Fvco=(4x126)MHz - else // Or calculate MSi on Fvco=750MHz - vfo[1].msi = (uint8_t)(750000000UL / (vfo[1].freq * vfo[1].ri)) & 0x000000fe; + // Set MSi + if (freq < 6000000UL) // Handle Low end of Ri=1 range + vfo[i].msi = (uint8_t)126; // Maximum MSi on Fvco=(4x126)MHz + else // Or calculate MSi on Fvco=750MHz + vfo[i].msi = (uint8_t)((700000000UL / (freq * vfo[i].ri)) & 0x000000fe); - msn = (double)(vfo[1].msi); // Re-calculate MSN - msn = msn * (double)(vfo[1].ri); - msn = msn * (double)(vfo[1].freq) / SI_XTAL_FREQ; - vfo[1].msn = msn; - - si_setmsn(1); - si_setmsi(1); - } - vfo[1].flag = 0; + msn = (double)(vfo[i].msi); // Re-calculate MSN + msn = msn * (double)(vfo[i].ri); + msn = msn * (double)(vfo[i].freq) / SI_XTAL_FREQ; + vfo[i].msn = msn; + + vfo[i].phase = (i==1)?PH000:PH090; // Hard coded phase + + si_setmsn(i); + si_setmsi(i); } + + vfo[i].freq = freq; // Adopt new freq } @@ -497,14 +468,12 @@ void si_init(void) // Initialize VFO values vfo[0].freq = 7074000; - vfo[0].flag = 0; vfo[0].phase = PH090; vfo[0].ri = 1; vfo[0].msi = 106; vfo[0].msn = ((double)vfo[0].freq*vfo[0].msi)/(double)SI_XTAL_FREQ; vfo[1].freq = 10000000; - vfo[1].flag = 0; vfo[1].phase = PH000; vfo[1].ri = 1; vfo[1].msi = 76; diff --git a/si5351.h b/si5351.h index 2b9e3d3..7db80b2 100644 --- a/si5351.h +++ b/si5351.h @@ -30,7 +30,6 @@ typedef struct { uint32_t freq; // type can hold up to 4GHz - uint8_t flag; // flag != 0 when update needed uint8_t phase; // in quarter waves (0, 1, 2, 3) uint8_t ri; // Ri (1 .. 128), but should be 1 for VFO 0 uint8_t msi; // MSi parameter a (4, 6, 8 .. 126) @@ -38,12 +37,12 @@ typedef struct } vfo_t; -int si_getreg(uint8_t *data, uint8_t reg, uint8_t len); -void si_init(void); -void si_evaluate(void); -void si_setfreq(int i, uint32_t f); +int si_getreg(uint8_t *buffer, uint8_t reg, uint8_t len); +int si_getvfo(int i, vfo_t *v); void si_setphase(int i, uint8_t p); void si_enable(int i, bool en); +void si_init(void); +void si_evaluate(int i, uint32_t freq); #endif /* _SI5351_H */ \ No newline at end of file diff --git a/uSDR.c b/uSDR.c index 88d57a8..ebe0bbf 100644 --- a/uSDR.c +++ b/uSDR.c @@ -86,6 +86,7 @@ int main() * i2c0 is used for the si5351 interface * i2c1 is used for the LCD and all other interfaces * if the display cannot keep up, try lowering the i2c1 frequency + * Do not invoke i2c using functions from interrupt handlers! */ i2c_init(i2c0, 400000); // i2c0 initialisation at 400Khz gpio_set_function(I2C0_SDA, GPIO_FUNC_I2C); @@ -112,9 +113,8 @@ int main() while (1) { sem_acquire_blocking(&loop_sem); // Wait until timer callback releases sem + hmi_evaluate(); // Refresh HMI (and VFO, BPF, etc) mon_evaluate(); // Check monitor input - si_evaluate(); // Refresh VFO settings - hmi_evaluate(); // Refresh HMI } return 0;