From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bin Meng <bmeng.cn@gmail.com>
Date: Sun, 8 Dec 2019 16:13:08 +0800
Subject: [PATCH v6 088/102] x86: apl: Add systemagent driver
In-Reply-To: <20191207044315.51770-7-sjg@chromium.org>
References: <20191207044315.51770-1-sjg@chromium.org>
 <20191207044315.51770-7-sjg@chromium.org>
Message-ID: <CAEUhbmVFsotvrusOg+PrAHYa8kK-21zGWEZQ3w7YyXYjo_RguQ@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 Sat, Dec 7, 2019 at 12:54 PM Simon Glass <sjg@chromium.org> wrote:
>
> This driver handles communication with the systemagent which needs to be
> told when U-Boot has completed its init.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> ---
>
> Changes in v6: None
> Changes in v5: None
> Changes in v4:
> - Add a comment for enable_bios_reset_cpl()
> - Tidy up header guards
> - use GENMASK() for VTBAR_MASK
>
> Changes in v3: None
> Changes in v2: None
>
>  arch/x86/cpu/apollolake/Makefile              |  2 +
>  arch/x86/cpu/apollolake/systemagent.c         | 19 ++++++++++
>  .../include/asm/arch-apollolake/systemagent.h | 37 +++++++++++++++++++
>  3 files changed, 58 insertions(+)
>  create mode 100644 arch/x86/cpu/apollolake/systemagent.c
>  create mode 100644 arch/x86/include/asm/arch-apollolake/systemagent.h
>
> diff --git a/arch/x86/cpu/apollolake/Makefile b/arch/x86/cpu/apollolake/Makefile
> index fdda748ea3..3a8c2f66a3 100644
> --- a/arch/x86/cpu/apollolake/Makefile
> +++ b/arch/x86/cpu/apollolake/Makefile
> @@ -2,5 +2,7 @@
>  #
>  # Copyright 2019 Google LLC
>
> +obj-$(CONFIG_SPL_BUILD) += systemagent.o
> +
>  obj-y += pmc.o
>  obj-y += uart.o
> diff --git a/arch/x86/cpu/apollolake/systemagent.c b/arch/x86/cpu/apollolake/systemagent.c
> new file mode 100644
> index 0000000000..3a41b329c3
> --- /dev/null
> +++ b/arch/x86/cpu/apollolake/systemagent.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2017 Intel Corporation.
> + * Take from coreboot project file of the same name
> + */
> +
> +#include <common.h>
> +#include <asm/intel_regs.h>
> +#include <asm/io.h>
> +#include <asm/arch/systemagent.h>
> +
> +void enable_bios_reset_cpl(void)
> +{
> +       /*
> +        * Set bits 0+1 of BIOS_RESET_CPL to indicate to the CPU
> +        * that BIOS has initialised memory and power management
> +        */

Could you put more comments here, like what you mentioned in the v5 comments:

"The FSP-S does not do it. If we leave this as zero then I believe the
power-aware interrupts don't work in Linux, and cpu 0 always gets the
interrupt."

> +       setbits_8(MCHBAR_REG(BIOS_RESET_CPL), 3);
> +}

[snip]

Regards,
Bin