Re: [Qemu-devel] [PATCH v6 2/5] Implement i.MX31 Clock Control Module

[Peter Maydell]

On 23 April 2012 00:31, Peter Chubb <address@hidden> wrote:
> +    /* Frequencies precalculated on register changes */
> +    uint32_t pll_refclk_freq;
> +    uint32_t mcu_clk_freq;
> +    uint32_t hsp_clk_freq;
> +    uint32_t ipg_clk_freq;
> +} IMXCCMState;
> +
> +static const VMStateDescription vmstate_imx_ccm = {
> +    .name = "imx-ccm",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(ccmr, IMXCCMState),
> +        VMSTATE_UINT32(pdr0, IMXCCMState),
> +        VMSTATE_UINT32(pdr1, IMXCCMState),
> +        VMSTATE_UINT32(mpctl, IMXCCMState),
> +        VMSTATE_UINT32(spctl, IMXCCMState),
> +        VMSTATE_UINT32_ARRAY(cgr, IMXCCMState, 3),
> +        VMSTATE_UINT32(pmcr0, IMXCCMState),
> +        VMSTATE_UINT32(pmcr1, IMXCCMState),
> +        VMSTATE_UINT32(pll_refclk_freq, IMXCCMState),
> +        VMSTATE_UINT32(mcu_clk_freq, IMXCCMState),
> +        VMSTATE_UINT32(hsp_clk_freq, IMXCCMState),
> +        VMSTATE_UINT32(ipg_clk_freq, IMXCCMState),

Rather than having the *_clk_freq saved and loaded
in the vmstate, I think it would be nicer to have a
post-load-hook that called update_clocks().

> +static uint32_t calc_pll(uint32_t pllreg, uint32_t base_freq)
> +{
> +    int32_t mfn = MFN(pllreg);  /* Numerator */
> +    uint32_t mfi = MFI(pllreg); /* Integer part */
> +    uint32_t mfd = 1 + MFD(pllreg); /* Denominator */
> +    uint32_t pd = 1 + PD(pllreg);   /* Pre-divider */
> +
> +    if (mfi < 5) {
> +        mfi = 5;
> +    }
> +    /* mfn is 10-bit signed twos-complement */
> +    mfn -= (mfn & 0x200);

What is this calculation supposed to do? It doesn't
convert a 10-bit signed twos-complement number into an
int32_t, unless I'm confused... Also, it's a rather
opaque way to write "mfn &= 0x200;".

-- PMM