> > 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 +1686,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); > > Is this delay really necessary? It makes very little sense to me to > add a delay here. We've switched back to the PLL, and whether we spin > for a bit or actually execute useful code whouldn't matter...
Hello, In the datasheet, in the description of CPUX_AXI_CFG_REG it reads "if the clock source is changed, at most to wait for 8 present running clock cycles." It doesn't say why do you have to wait, nor _for_what_ do you have to wait. So I assume it refers to not interact with other components of the soc until at least 8 nop operations. I put those delay() to notice this, although as I said before I don't know if this would be the best choice. For cases like this maybe it would be better to define a delay macro in cycles with some inline assembly. I don't know if this is really necessary here, but if you don't include such a delay I think it would be a good idea to notice this in a comment, so in the future if the code gets more complicated a possible bug could be sprayed. Regards, adr.
