On 26 January 2016 at 21:44, Jean-Christophe Dubois <j...@tribudubois.net>
wrote:
> Various i.MX timers (GPT, EPIT, PWM, ...) are only requesting 4 clocks
> from the system.
> * CLK_NONE,
> * CLK_IPG,
> * CLK_IPG_HIGH,
> * CLK_32k
>
> Other "clocks" are not required by the qemu framework so far.
This patch confuses me. You don't say why we're removing a bunch
of code or why it's OK to do that.
> Signed-off-by: Jean-Christophe Dubois <j...@tribudubois.net>
> ---
> hw/misc/imx25_ccm.c | 35 +++++------------------------------
> hw/misc/imx31_ccm.c | 38 ++++----------------------------------
> hw/timer/imx_epit.c | 8 ++++----
> hw/timer/imx_gpt.c | 16 ++++++++--------
> include/hw/misc/imx_ccm.h | 10 ++--------
> 5 files changed, 23 insertions(+), 84 deletions(-)
>
> diff --git a/hw/misc/imx25_ccm.c b/hw/misc/imx25_ccm.c
> index 23759ca..38599f2 100644
> --- a/hw/misc/imx25_ccm.c
> +++ b/hw/misc/imx25_ccm.c
> @@ -119,20 +119,6 @@ static uint32_t imx25_ccm_get_mpll_clk(IMXCCMState *dev)
> return freq;
> }
>
> -static uint32_t imx25_ccm_get_upll_clk(IMXCCMState *dev)
> -{
> - uint32_t freq = 0;
> - IMX25CCMState *s = IMX25_CCM(dev);
> -
> - if (!EXTRACT(s->reg[IMX25_CCM_CCTL_REG], UPLL_DIS)) {
> - freq = imx_ccm_calc_pll(s->reg[IMX25_CCM_UPCTL_REG], CKIH_FREQ);
> - }
> -
> - DPRINTF("freq = %d\n", freq);
> -
> - return freq;
> -}
> -
> static uint32_t imx25_ccm_get_mcu_clk(IMXCCMState *dev)
> {
> uint32_t freq;
> @@ -181,21 +167,10 @@ static uint32_t
> imx25_ccm_get_clock_frequency(IMXCCMState *dev, IMXClk clock)
> DPRINTF("Clock = %d)\n", clock);
>
> switch (clock) {
> - case NOCLK:
> - break;
> - case CLK_MPLL:
> - freq = imx25_ccm_get_mpll_clk(dev);
> - break;
> - case CLK_UPLL:
> - freq = imx25_ccm_get_upll_clk(dev);
> - break;
> - case CLK_MCU:
> - freq = imx25_ccm_get_mcu_clk(dev);
> - break;
> - case CLK_AHB:
> - freq = imx25_ccm_get_ahb_clk(dev);
> + case CLK_NONE:
> break;
> case CLK_IPG:
> + case CLK_IPG_HIGH:
> freq = imx25_ccm_get_ipg_clk(dev);
> break;
> case CLK_32k:
> @@ -221,9 +196,9 @@ static void imx25_ccm_reset(DeviceState *dev)
> memset(s->reg, 0, IMX25_CCM_MAX_REG * sizeof(uint32_t));
> s->reg[IMX25_CCM_MPCTL_REG] = 0x800b2c01;
> s->reg[IMX25_CCM_UPCTL_REG] = 0x84042800;
> - /*
> + /*
> * The value below gives:
> - * CPU = 133 MHz, AHB = 66,5 MHz, IPG = 33 MHz.
> + * CPU = 133 MHz, AHB = 66,5 MHz, IPG = 33 MHz.
> */
> s->reg[IMX25_CCM_CCTL_REG] = 0xd0030000;
> s->reg[IMX25_CCM_CGCR0_REG] = 0x028A0100;
> @@ -240,7 +215,7 @@ static void imx25_ccm_reset(DeviceState *dev)
>
> /*
> * default boot will change the reset values to allow:
> - * CPU = 399 MHz, AHB = 133 MHz, IPG = 66,5 MHz.
> + * CPU = 399 MHz, AHB = 133 MHz, IPG = 66,5 MHz.
> * For some reason, this doesn't work. With the value below, linux
> * detects a 88 MHz IPG CLK instead of 66,5 MHz.
> s->reg[IMX25_CCM_CCTL_REG] = 0x20032000;
> diff --git a/hw/misc/imx31_ccm.c b/hw/misc/imx31_ccm.c
> index 47d6ead..3876cc2 100644
> --- a/hw/misc/imx31_ccm.c
> +++ b/hw/misc/imx31_ccm.c
> @@ -111,7 +111,7 @@ static uint32_t imx31_ccm_get_pll_ref_clk(IMXCCMState
> *dev)
> if (s->reg[IMX31_CCM_CCMR_REG] & CCMR_FPMF) {
> freq *= 1024;
> }
> - }
> + }
What are all these whitespace only changes? If you need to do a
whitespace cleanup please put it in its own patch.
> case CLK_32k:
> diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
> index 50bf83c..a67bcb5 100644
> --- a/hw/timer/imx_epit.c
> +++ b/hw/timer/imx_epit.c
> @@ -51,10 +51,10 @@ static char const *imx_epit_reg_name(uint32_t reg)
> * These are typical.
> */
> static const IMXClk imx_epit_clocks[] = {
> - NOCLK, /* 00 disabled */
> - CLK_IPG, /* 01 ipg_clk, ~532MHz */
> - CLK_IPG, /* 10 ipg_clk_highfreq */
> - CLK_32k, /* 11 ipg_clk_32k -- ~32kHz */
> + CLK_NONE, /* 00 disabled */
> + CLK_IPG, /* 01 ipg_clk, ~532MHz */
> + CLK_IPG_HIGH, /* 10 ipg_clk_highfreq */
> + CLK_32k, /* 11 ipg_clk_32k -- ~32kHz */
> };
>
> /*
> diff --git a/hw/timer/imx_gpt.c b/hw/timer/imx_gpt.c
> index b227256..b2f03b0 100644
> --- a/hw/timer/imx_gpt.c
> +++ b/hw/timer/imx_gpt.c
> @@ -80,14 +80,14 @@ static const VMStateDescription vmstate_imx_timer_gpt = {
> };
>
> static const IMXClk imx_gpt_clocks[] = {
> - NOCLK, /* 000 No clock source */
> - CLK_IPG, /* 001 ipg_clk, 532MHz*/
> - CLK_IPG, /* 010 ipg_clk_highfreq */
> - NOCLK, /* 011 not defined */
> - CLK_32k, /* 100 ipg_clk_32k */
> - NOCLK, /* 101 not defined */
> - NOCLK, /* 110 not defined */
> - NOCLK, /* 111 not defined */
> + CLK_NONE, /* 000 No clock source */
> + CLK_IPG, /* 001 ipg_clk, 532MHz*/
> + CLK_IPG_HIGH, /* 010 ipg_clk_highfreq */
> + CLK_NONE, /* 011 not defined */
> + CLK_32k, /* 100 ipg_clk_32k */
> + CLK_NONE, /* 101 not defined */
> + CLK_NONE, /* 110 not defined */
> + CLK_NONE, /* 111 not defined */
> };
These are just renaming NOCLK to CLK_NONE and fixing formatting?
Again, please don't put that in the same patch as substantive
code changes.
thanks
-- PMM
