[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH 1/3] aspeed/scu: introduce clock frequencies
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-arm] [PATCH 1/3] aspeed/scu: introduce clock frequencies |
Date: |
Fri, 22 Jun 2018 07:28:04 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
On 06/22/2018 02:57 AM, Joel Stanley wrote:
> On 22 June 2018 at 08:09, Cédric Le Goater <address@hidden> 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 <address@hidden>
>> ---
>> 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.
yes. I will fix that.
Thanks,
C.
>> + freq_select = SCU_AST2400_HW_STRAP_GET_H_PLL_CLK(s->hw_strap1);
>> +
>> + return hpll_ast2400_freqs[clk_25m_in][freq_select] * 1000000;
>> +}