[U-Boot] [PATCH v5 079/101] x86: Add low-power subsystem (lpss) support

[Simon Glass]

Hi Bin,

On Thu, 28 Nov 2019 at 22:49, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Nov 25, 2019 at 12:12 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > This subsystem is present on various Intel SoCs.
> >
> > Add very basic support for taking an lpss device out of reset.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> >
> > ---
> >
> > Changes in v5: None
> > Changes in v4:
> > - Add support for updating power state
> > - Move this to intel_common
> >
> > Changes in v3: None
> > Changes in v2: None
> >
> >  arch/x86/cpu/intel_common/Makefile |  1 +
> >  arch/x86/cpu/intel_common/lpss.c   | 44 ++++++++++++++++++++++++++++++
> >  arch/x86/include/asm/lpss.h        | 36 ++++++++++++++++++++++++
> >  3 files changed, 81 insertions(+)
> >  create mode 100644 arch/x86/cpu/intel_common/lpss.c
> >  create mode 100644 arch/x86/include/asm/lpss.h
> >
> > diff --git a/arch/x86/cpu/intel_common/Makefile b/arch/x86/cpu/intel_common/Makefile
> > index 09212cee04..cc4e1c962b 100644
> > --- a/arch/x86/cpu/intel_common/Makefile
> > +++ b/arch/x86/cpu/intel_common/Makefile
> > @@ -19,6 +19,7 @@ endif
> >  obj-y += cpu.o
> >  obj-y += fast_spi.o
> >  obj-y += lpc.o
> > +obj-y += lpss.o
> >  ifndef CONFIG_TARGET_EFI_APP
> >  obj-$(CONFIG_$(SPL_TPL_)X86_32BIT_INIT) += microcode.o
> >  ifndef CONFIG_$(SPL_)X86_64
> > diff --git a/arch/x86/cpu/intel_common/lpss.c b/arch/x86/cpu/intel_common/lpss.c
> > new file mode 100644
> > index 0000000000..26a2d2d1e3
> > --- /dev/null
> > +++ b/arch/x86/cpu/intel_common/lpss.c
> > @@ -0,0 +1,44 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Special driver to handle of-platdata
> > + *
> > + * Copyright 2019 Google LLC
> > + *
> > + * Some code from coreboot lpss.c
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <pci.h>
> > +#include <asm/io.h>
> > +#include <asm/lpss.h>
> > +
> > +enum {
> > +       LPSS_RESET_CTL_REG      = 0x204,
> > +
> > +       /*
> > +        * Bit 1:0 controls LPSS controller reset.
> > +        *
> > +        * 00 ->LPSS Host Controller is in reset (Reset Asserted)
> > +        * 01/10 ->Reserved
> > +        * 11 ->LPSS Host Controller is NOT at reset (Reset Released)
> > +        */
> > +       LPSS_CNT_RST_RELEASE    = 3,
> > +
> > +       /* Power management control and status register */
> > +       PME_CTRL_STATUS         = 0x84,
> > +
> > +       /* Bit 1:0 Powerstate, controls D0 and D3 state */
> > +       POWER_STATE_MASK        = 3,
>
> Could we use #defines? These macros are register offsets and bit
> defines and should not be put in one enum.

Well we can, but I think enums are better. The enum is anonymous so it
can't really be referenced by anything. This is just a 'C' way of
doing constants, I think.

Regards,
Simon