Hi Bin,
On Wed, 9 Oct 2019 at 21:11, Bin Meng <bmeng...@gmail.com> wrote:
>
> Hi Simon,
>
> On Wed, Sep 25, 2019 at 10:58 PM Simon Glass <s...@chromium.org> wrote:
> >
> > Intel x86 SoCs have a power manager/controller which handles several
> > power-related aspects of the platform. Add a uclass for this, with a few
> > useful operations.
> >
>
> I don't like to create another x86 specific uclass for this power
> management controller.
>
> I see two options:
>
> - Create a standard acpi-pmc uclass instead. Almost all registers in
> the PMC module follow the ACPI spec, except a few
> - Do not abstract this to a uclass level. Use platform codes to do
> such. x86 SoC varies from one to another, and it's very hard to do a
> one-size-fit-all job.
I like the first option, at least for now. I suppose as we see more
SoCs added it might get tricky, but quite a few seem to use this
common approach.
>
> > Signed-off-by: Simon Glass <s...@chromium.org>
> > ---
> >
> > drivers/power/Kconfig | 2 +
> > drivers/power/power_mgr/Kconfig | 25 +++
> > drivers/power/power_mgr/Makefile | 6 +
> > drivers/power/power_mgr/power-mgr-uclass.c | 191 +++++++++++++++++++++
> > include/dm/uclass-id.h | 1 +
> > include/power/power_mgr.h | 177 +++++++++++++++++++
> > 6 files changed, 402 insertions(+)
> > create mode 100644 drivers/power/power_mgr/Kconfig
> > create mode 100644 drivers/power/power_mgr/Makefile
> > create mode 100644 drivers/power/power_mgr/power-mgr-uclass.c
> > create mode 100644 include/power/power_mgr.h
> >
> > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> > index 9495dca33b9..1cb3f6d5e25 100644
> > --- a/drivers/power/Kconfig
> > +++ b/drivers/power/Kconfig
> > @@ -4,6 +4,8 @@ source "drivers/power/domain/Kconfig"
> >
> > source "drivers/power/pmic/Kconfig"
> >
> > +source "drivers/power/power_mgr/Kconfig"
> > +
> > source "drivers/power/regulator/Kconfig"
> >
> > choice
> > diff --git a/drivers/power/power_mgr/Kconfig
> > b/drivers/power/power_mgr/Kconfig
> > new file mode 100644
> > index 00000000000..2731518462f
> > --- /dev/null
> > +++ b/drivers/power/power_mgr/Kconfig
> > @@ -0,0 +1,25 @@
> > +config POWER_MGR
> > + bool "Power Manager (x86 PMC) support"
> > + help
> > + Enable support for an x86-style power-management controller which
> > + provides features including checking whether the system started
> > from
> > + resume, powering off the system and enabling/disabling the reset
> > + mechanism.
> > +
> > +config SPL_POWER_MGR
> > + bool "Power Manager (x86 PMC) support in SPL"
> > + default y if POWER_MGR
> > + help
> > + Enable support for an x86-style power-management controller which
> > + provides features including checking whether the system started
> > from
> > + resume, powering off the system and enabling/disabling the reset
> > + mechanism.
> > +
> > +config TPL_POWER_MGR
> > + bool "Power Manager (x86 PMC) support in TPL"
>
> Why would we need access PMC in both SPL and TPL?
The code used in TPL is in arch_cpu_init_tpm():
- enable the fixed BAR
- clear global reset promotion
- disable TCO
>
> > + default y if POWER_MGR
> > + help
> > + Enable support for an x86-style power-management controller which
> > + provides features including checking whether the system started
> > from
> > + resume, powering off the system and enabling/disabling the reset
> > + mechanism.
> > diff --git a/drivers/power/power_mgr/Makefile
> > b/drivers/power/power_mgr/Makefile
> > new file mode 100644
> > index 00000000000..87542f5248a
> > --- /dev/null
> > +++ b/drivers/power/power_mgr/Makefile
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright 2019 Google LLC
> > +
> > +obj-$(CONFIG_$(SPL_TPL_)POWER_MGR) += power-mgr-uclass.o
> > +
> > diff --git a/drivers/power/power_mgr/power-mgr-uclass.c
> > b/drivers/power/power_mgr/power-mgr-uclass.c
> > new file mode 100644
> > index 00000000000..0d73caf8cf6
> > --- /dev/null
> > +++ b/drivers/power/power_mgr/power-mgr-uclass.c
> > @@ -0,0 +1,191 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2019 Google LLC
> > + */
> > +
> > +#define LOG_CATEGORY UCLASS_POWER_MGR
> > +
> > +#include <common.h>
> > +#include <acpi_s3.h>
> > +#include <dm.h>
> > +#include <log.h>
> > +#include <asm/io.h>
> > +#ifdef CONFIG_CREATE_ARCH_SYMLINK
> > +#include <asm/arch/gpio.h>
> > +#endif
> > +#include <power/power_mgr.h>
> > +
> > +enum {
> > + PM1_STS = 0x00,
> > + PM1_EN = 0x02,
> > + PM1_CNT = 0x04,
> > +
> > + GPE0_STS = 0x20,
> > + GPE0_EN = 0x30,
> > +};
> > +
> > +struct tco_regs {
> > + u32 tco_rld;
> > + u32 tco_sts;
> > + u32 tco1_cnt;
> > + u32 tco_tmr;
> > +};
> > +
> > +enum {
> > + TCO_STS_TIMEOUT = 1 << 3,
> > + TCO_STS_SECOND_TO_STS = 1 << 17,
> > + TCO1_CNT_HLT = 1 << 11,
> > +};
> > +
> > +static void pmc_fill_pm_reg_info(struct udevice *dev)
> > +{
> > + struct power_mgr_upriv *upriv = dev_get_uclass_priv(dev);
> > + int i;
> > +
> > + upriv->pm1_sts = inw(upriv->acpi_base + PM1_STS);
> > + upriv->pm1_en = inw(upriv->acpi_base + PM1_EN);
> > + upriv->pm1_cnt = inw(upriv->acpi_base + PM1_CNT);
> > +
> > + log_debug("pm1_sts: %04x pm1_en: %04x pm1_cnt: %08x\n",
> > + upriv->pm1_sts, upriv->pm1_en, upriv->pm1_cnt);
> > +
> > + for (i = 0; i < GPE0_REG_MAX; i++) {
> > + upriv->gpe0_sts[i] = inl(upriv->acpi_base + GPE0_STS + i *
> > 4);
> > + upriv->gpe0_en[i] = inl(upriv->acpi_base + GPE0_EN + i * 4);
> > + log_debug("gpe0_sts[%d]: %08x gpe0_en[%d]: %08x\n", i,
> > + upriv->gpe0_sts[i], i, upriv->gpe0_en[i]);
> > + }
> > +}
> > +
> > +int pmc_disable_tco_base(ulong tco_base)
> > +{
>
> TCO is not defined in the ACPI spec. This is APL specific. It looks
> this has something to do with the hardware watchdog?
>
> Anyway, if we want to put such in an acpi-pmc uclass driver, we should
> mark such op as optional.
OK.
Regards,
SImon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
