On 22 June 2018 at 08:09, Cédric Le Goater <c...@kaod.org> wrote: > All Aspeed SoC clocks are driven by an input source clock which can > have different frequencies : 24MHz or 25MHz, and also, on the Aspeed > AST2400 SoC, 48MHz. The H-PLL (CPU) clock is defined from a > calculation using parameters in the H-PLL Parameter register or from a > predefined set of frequencies if the setting is strapped by hardware > (Aspeed AST2400 SoC). The other clocks of the SoC are then defined > from the H-PLL using dividers. > > We introduce first the APB clock because it drives the timer.
Looks good! One small issue below. > > Signed-off-by: Cédric Le Goater <c...@kaod.org> > --- > include/hw/misc/aspeed_scu.h | 70 ++++++++++++++++++++++++++-- > hw/misc/aspeed_scu.c | 106 > +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 172 insertions(+), 4 deletions(-) > > +static uint32_t aspeed_scu_calc_hpll_ast2400(AspeedSCUState *s) > +{ > + uint32_t hpll_reg = s->regs[HPLL_PARAM]; > + uint8_t freq_select; > + bool clk_25m_in; > + > + if (hpll_reg & SCU_AST2400_H_PLL_OFF) { > + return 0; > + } > + > + if (hpll_reg & SCU_AST2400_H_PLL_PROGRAMMED) { > + uint32_t multiplier = 1; > + > + if (!(hpll_reg & SCU_AST2400_H_PLL_BYPASS_EN)) { > + uint32_t n = (hpll_reg >> 5) & 0x3f; > + uint32_t od = (hpll_reg >> 4) & 0x1; > + uint32_t d = hpll_reg & 0xf; > + > + multiplier = (2 - od) * ((n + 2) / (d + 1)); > + } > + > + return s->clkin * multiplier; > + } > + > + /* HW strapping */ > + clk_25m_in = (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN); I think you want to do !! to this result, or shift it down. Otherwise you are getting 1 << 23 or zero, when I think you want 1 or 0. > + freq_select = SCU_AST2400_HW_STRAP_GET_H_PLL_CLK(s->hw_strap1); > + > + return hpll_ast2400_freqs[clk_25m_in][freq_select] * 1000000; > +}