[U-Boot] [PATCH v3 033/108] x86: power: Add an ACPI PMC uclass

[Simon Glass]

Hi Bin,

On Mon, 4 Nov 2019 at 00:03, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Oct 21, 2019 at 11:40 AM Simon Glass <sjg at 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.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v3:
> > - Rename power-mgr uclass to acpi-pmc
> >
> > Changes in v2: None
> >
> >  drivers/power/Kconfig                    |   2 +
> >  drivers/power/acpi_pmc/Kconfig           |  25 +++
> >  drivers/power/acpi_pmc/Makefile          |   5 +
> >  drivers/power/acpi_pmc/acpi-pmc-uclass.c | 191 +++++++++++++++++++++++
> >  include/dm/uclass-id.h                   |   1 +
> >  include/power/acpi_pmc.h                 | 185 ++++++++++++++++++++++
> >  6 files changed, 409 insertions(+)
> >  create mode 100644 drivers/power/acpi_pmc/Kconfig
> >  create mode 100644 drivers/power/acpi_pmc/Makefile
> >  create mode 100644 drivers/power/acpi_pmc/acpi-pmc-uclass.c
> >  create mode 100644 include/power/acpi_pmc.h
> >
[..]

> > +int pmc_init(struct udevice *dev)
> > +{
> > +       const struct acpi_pmc_ops *ops = acpi_pmc_get_ops(dev);
> > +       int ret;
> > +
> > +       pmc_fill_pm_reg_info(dev);
> > +       if (!ops->init)
> > +               return -ENOSYS;
> > +
> > +       ret = ops->init(dev);
> > +       if (ret)
> > +               return log_msg_ret("Failed to init pmc", ret);
> > +
> > +#ifdef DEBUG
> > +       pmc_dump_info(dev);
> > +#endif
> > +
> > +       return 0;
> > +}
>
> I wonder shouldn't this be covered by the probe() method?

This is called at present from SPL to read information that is not
needed in TPL. In effect the PMC is mostly just used for info in TPL,
so we don't want to

We could have a check in the probe() function and do different things
in TPL, but at the moment I feel that it is worth having an 'init'
function which actually sets things up when needed.

[..]

>
> [snip]
>
> Regards,
> Bin