Hello, sorry for late reply. > It seems that the PLL is not been stabilized although LOCK is read as > set. You can check it by putting a delay in sxiccmu_h3_set_frequency > after H3_PLL_CPUX_LOCK is tested, delay(200) is sufficient, the machine > will boot ok.
My OrangePi PC took 257usec to LOCK, this is not so different from the value of PLL_STABLE_TIME_REG1 register (default: 255). I thought LOCK flag is set regardless PLL is locked or not, simply time passed... but your comment tells the flag reflects PLL status but unstable. > Using a divider seams to make the stabilizing faster, but this > random behavior is a little tricky to make a solution of it, and > in the future other problems could arise. I thought M=2 first because this U-boot patch uses the value. https://github.com/aclex/linux/blob/orange-pi-4.15/0001-sunxi-h3-Fix-PLL1-setup-to-never-use-dividers.patch But recently I found Linux guys say that M=1 is more stable. https://github.com/megous/linux/commit/88be3d421e958579026135d8acec4b3983958738 sxiccmu.c already uses M=1 and P=1 so modifing K and N calculator code (my diff) is not good. > What about gating the PLL before applying the multipliers and ungating > it afterwards? This works very good on my OrangePi PC. Original code (M=1) hanged up at boot, but this PLL-gating resolved that problem! > Note that the datasheet warns about the need of waiting 8 cycles of the > current clock after changing CPUX_CLK_SRC_SEL, So it may be helpful to > add a small delay after setting CPUX_CLK_SRC_SEL. I put delay(1) as a > reference, I'm starting to look under the hood of OpenBSD, I don't know > what is the preffered way to deal with such a small delay in cycles in > this place. I think putting delay(1) is no problem. Other code such as amlclock.c uses delay(100) after switching clock multiplexer. Everyone does not want to wait but this is needed for stability. Here is a modified version of your diff. How about this? Index: sxiccmu.c =================================================================== RCS file: /cvs/src/sys/dev/fdt/sxiccmu.c,v retrieving revision 1.27 diff -u -p -r1.27 sxiccmu.c --- sxiccmu.c 28 Mar 2020 12:32:53 -0000 1.27 +++ sxiccmu.c 17 Aug 2020 11:38:05 -0000 @@ -1158,6 +1158,7 @@ sxiccmu_a80_get_frequency(struct sxiccmu /* Allwinner H3/H5 */ #define H3_PLL_CPUX_CTRL_REG 0x0000 +#define H3_PLL_CPUX_ENABLE (1 << 31) #define H3_PLL_CPUX_LOCK (1 << 28) #define H3_PLL_CPUX_OUT_EXT_DIVP(x) (((x) >> 16) & 0x3) #define H3_PLL_CPUX_OUT_EXT_DIVP_MASK (0x3 << 16) @@ -1176,6 +1177,8 @@ sxiccmu_a80_get_frequency(struct sxiccmu #define H3_CPUX_CLK_SRC_SEL_LOSC (0x0 << 16) #define H3_CPUX_CLK_SRC_SEL_OSC24M (0x1 << 16) #define H3_CPUX_CLK_SRC_SEL_PLL_CPUX (0x2 << 16) +#define H3_PLL_STABLE_TIME_REG1 0x0204 +#define H3_PLL_STABLE_TIME_REG1_TIME(x) (((x) >> 0) & 0xffff) uint32_t sxiccmu_h3_get_frequency(struct sxiccmu_softc *sc, uint32_t idx) @@ -1632,7 +1635,7 @@ sxiccmu_h3_set_frequency(struct sxiccmu_ { struct sxiccmu_clock clock; uint32_t parent, parent_freq; - uint32_t reg; + uint32_t reg, lock_time; int k, n; int error; @@ -1644,7 +1647,12 @@ sxiccmu_h3_set_frequency(struct sxiccmu_ while (n >= 1 && (24000000 * n * k) > freq) n--; + /* Gate the PLL first */ reg = SXIREAD4(sc, H3_PLL_CPUX_CTRL_REG); + reg &= ~H3_PLL_CPUX_ENABLE; + SXIWRITE4(sc, H3_PLL_CPUX_CTRL_REG, reg); + + /* Set factors and external divider. */ reg &= ~H3_PLL_CPUX_OUT_EXT_DIVP_MASK; reg &= ~H3_PLL_CPUX_FACTOR_N_MASK; reg &= ~H3_PLL_CPUX_FACTOR_K_MASK; @@ -1653,7 +1661,15 @@ sxiccmu_h3_set_frequency(struct sxiccmu_ reg |= ((k - 1) << H3_PLL_CPUX_FACTOR_K_SHIFT); SXIWRITE4(sc, H3_PLL_CPUX_CTRL_REG, reg); - /* Wait for PLL to lock. */ + /* Ungate the PLL */ + reg |= H3_PLL_CPUX_ENABLE; + SXIWRITE4(sc, H3_PLL_CPUX_CTRL_REG, reg); + + /* delay for PLL stable (LOCK flag is unreliable) */ + lock_time = SXIREAD4(sc, H3_PLL_STABLE_TIME_REG1); + delay(H3_PLL_STABLE_TIME_REG1_TIME(lock_time)); + + /* Check LOCK flag */ while ((SXIREAD4(sc, H3_PLL_CPUX_CTRL_REG) & H3_PLL_CPUX_LOCK) == 0) delay(1); @@ -1665,6 +1681,8 @@ sxiccmu_h3_set_frequency(struct sxiccmu_ reg &= ~H3_CPUX_CLK_SRC_SEL; reg |= H3_CPUX_CLK_SRC_SEL_OSC24M; SXIWRITE4(sc, H3_CPUX_AXI_CFG_REG, reg); + /* Must wait at least 8 cycles of the current clock. */ + delay(1); error = sxiccmu_h3_set_frequency(sc, H3_CLK_PLL_CPUX, freq); @@ -1673,6 +1691,7 @@ sxiccmu_h3_set_frequency(struct sxiccmu_ reg &= ~H3_CPUX_CLK_SRC_SEL; reg |= H3_CPUX_CLK_SRC_SEL_PLL_CPUX; SXIWRITE4(sc, H3_CPUX_AXI_CFG_REG, reg); + delay(1); return error; case H3_CLK_MMC0: case H3_CLK_MMC1: ---- best regards, -- SASANO Takayoshi (JG1UAA) <u...@cvs.openbsd.org>
