From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bin Meng <bmeng.cn@gmail.com>
Date: Wed, 9 Oct 2019 22:01:50 +0800
Subject: [U-Boot] [PATCH 066/126] x86: spl: Support init of a PUNIT
In-Reply-To: <20190925145750.200592-67-sjg@chromium.org>
References: <20190925145750.200592-1-sjg@chromium.org>
 <20190925145750.200592-67-sjg@chromium.org>
Message-ID: <CAEUhbmXwd++12mMJbhOpX4CeQLN7sFNho5cAb2oEcHs0o+0pnA@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Simon,

On Wed, Sep 25, 2019 at 10:58 PM Simon Glass <sjg@chromium.org> wrote:
>
> The x86 power unit handles power management. Support initing this device
> which is modelled as a new type of system controller since there are no
> operations needed.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/x86/include/asm/cpu.h |  1 +
>  arch/x86/lib/spl.c         | 40 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
>
> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> index feee0f915f6..21a05dab7de 100644
> --- a/arch/x86/include/asm/cpu.h
> +++ b/arch/x86/include/asm/cpu.h
> @@ -55,6 +55,7 @@ enum {
>         X86_SYSCON_PINCONF,     /* Intel x86 pin configuration */
>         X86_SYSCON_PMU,         /* Power Management Unit */
>         X86_SYSCON_SCU,         /* System Controller Unit */
> +       X86_SYSCON_PUNIT,       /* Power unit */
>  };
>
>  struct cpuid_result {
> diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
> index 2baac913837..db1ce67a590 100644
> --- a/arch/x86/lib/spl.c
> +++ b/arch/x86/lib/spl.c
> @@ -5,11 +5,15 @@
>
>  #include <common.h>
>  #include <debug_uart.h>
> +#include <dm.h>
>  #include <malloc.h>
>  #include <spl.h>
> +#include <syscon.h>
>  #include <asm/cpu.h>
> +#include <asm/cpu_common.h>
>  #include <asm/mrccache.h>
>  #include <asm/mtrr.h>
> +#include <asm/pci.h>
>  #include <asm/processor.h>
>  #include <asm/spl.h>
>  #include <asm-generic/sections.h>
> @@ -21,6 +25,32 @@ __weak int arch_cpu_init_dm(void)
>         return 0;
>  }
>
> +#ifdef CONFIG_TPL
> +
> +static int set_max_freq(void)
> +{
> +       if (cpu_get_burst_mode_state() == BURST_MODE_UNAVAILABLE) {
> +               /*
> +                * Burst Mode has been factory-configured as disabled and is not
> +                * available in this physical processor package
> +                */
> +               debug("Burst Mode is factory-disabled\n");
> +               return -ENOENT;
> +       }
> +
> +       /* Enable burst mode */
> +       cpu_set_burst_mode(true);
> +
> +       /* Enable speed step */
> +       cpu_set_eist(true);
> +
> +       /* Set P-State ratio */
> +       cpu_set_p_state_to_turbo_ratio();
> +
> +       return 0;
> +}
> +#endif
> +
>  static int x86_spl_init(void)
>  {
>  #ifndef CONFIG_TPL
> @@ -31,6 +61,8 @@ static int x86_spl_init(void)
>          * place it immediately below CONFIG_SYS_TEXT_BASE.
>          */
>         char *ptr = (char *)0x110000;
> +#else
> +       struct udevice *punit;
>  #endif
>         int ret;
>
> @@ -101,6 +133,14 @@ static int x86_spl_init(void)
>                 return ret;
>         }
>         mtrr_commit(true);
> +#else
> +       ret = syscon_get_by_driver_data(X86_SYSCON_PUNIT, &punit);
> +       if (ret)
> +               debug("Could not find PUNIT (err-%d)\n", ret);

err=%d

But I wonder what's the purpose of init punit at this point?

> +
> +       ret = set_max_freq();

Can we delay this to U-Boot proper? I don't see a need to bring the
core up to speed at this early stage.

> +       if (ret)
> +               debug("Failed to set CPU frequency (err=%d)\n", ret);
>  #endif
>
>         return 0;
> --

Regards,
Bin