[U-Boot] [PATCH 28/40] x86: Add a sysreset driver for the Intel PCH

[Bin Meng]

Hi Simon,

On Sat, Apr 6, 2019 at 10:02 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Bin,
>
> On Wed, 3 Apr 2019 at 02:42, Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Wed, Apr 3, 2019 at 10:22 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Fri, 22 Feb 2019 at 00:20, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Wed, Jan 30, 2019 at 12:00 PM Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Intel SoCs support a fairly stardard reset mechanism which can support
> > > > > powering off the device. Add support for this and enable it by default on
> > > > > broadwell, which already has the necessary pm.h header file.
> > > > >
> > > > > This driver augments the standard x86 sysreset driver.
> > > > >
> > > >
> > > > I think we need update the existing sysreset_x86 driver to support
> > > > SYSRESET_POWER_OFF, instead of creating a new driver to do such.
> > > >
> > > > We can add a new PCH ioctl code to get the pmbase, and do the ACPI
> > > > power off in the sysreset_x86 driver.
> > >
> > > So are you saying that the registers are the same for all x86 chips?
> > > From what I can tell, the offsets vary.
> >
> > Yes, the offsets might be different, so instead of getting only the
> > pmbase, we need do something like getting ACPI PM register block
> > information from the PCH driver. That's how acpi_create_fadt() passes
> > ACPI register block information to the OS.
>
> That doesn't make a lot of sense to me. If the register layout is
> different, why not just use different drivers?
>

Yes, the register layout might be different but the programming model
is the same and well defined in the ACPI spec.

> I suppose we could put the common code (with parameters) in a separate
> file and call it from the driver. The parameters would presumably be:
>
> PMBASE
> GPE0_EN(0)
> PM1_STS
> PM1_CNT

That's why I suggest to ask PCH driver to provide these offsets in a
struct via an ioctl call to the sysreset driver.

Regards,
Bin