[PATCH v3 00/57] dm: Add programatic generation of ACPI tables (part D)

classic Classic list List threaded Threaded
80 messages Options
1234
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices

Wolfgang Wallner
Hi Simon,

-----"Simon Glass" <[hidden email]> schrieb: -----

> Betreff: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
>
> Add common x86 ASL files, taken from coreboot.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v1)
>
>  arch/x86/include/asm/acpi/chromeos.asl    | 108 +++++++++++++++++
>  arch/x86/include/asm/acpi/cpu.asl         |  25 ++++
>  arch/x86/include/asm/acpi/cros_gnvs.asl   |  29 +++++
>  arch/x86/include/asm/acpi/lpc.asl         | 141 ++++++++++++++++++++++
>  arch/x86/include/asm/acpi/pci_osc.asl     |  21 ++++
>  arch/x86/include/asm/acpi/pcr.asl         |  80 ++++++++++++
>  arch/x86/include/asm/acpi/ramoops.asl     |  32 +++++
>  arch/x86/include/asm/acpi/sleepstates.asl |  12 +-
>  8 files changed, 443 insertions(+), 5 deletions(-)
>  create mode 100644 arch/x86/include/asm/acpi/chromeos.asl
>  create mode 100644 arch/x86/include/asm/acpi/cpu.asl
>  create mode 100644 arch/x86/include/asm/acpi/cros_gnvs.asl
>  create mode 100644 arch/x86/include/asm/acpi/lpc.asl
>  create mode 100644 arch/x86/include/asm/acpi/pci_osc.asl
>  create mode 100644 arch/x86/include/asm/acpi/pcr.asl
>  create mode 100644 arch/x86/include/asm/acpi/ramoops.asl

There are recent (~June 2020) commits in coreboot which convert most ASL files
to ASL 2.0. The ASL 2.0 syntax is much easier to read IMHO, and I think it
would be good to update this patch to take advantage of the recent coreboot
developments. See e.g. the commit at [1].

regards, Wolfgang

[1] https://github.com/coreboot/coreboot/commit/03248033e7be6f81ad5b60ed21a60071aee32c67



Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 05/57] x86: apl: Correct PCIE_ECAM_BASE

Bin Meng-2
In reply to this post by Simon Glass-3
On Mon, Sep 7, 2020 at 5:44 AM Simon Glass <[hidden email]> wrote:

>
> This value is incorrect and causes problems booting Linux. Fix it.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v1)
>
>  board/google/chromebook_coral/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 15/57] x86: link: Allow more space for U-Boot

Bin Meng-2
In reply to this post by Simon Glass-3
On Mon, Sep 7, 2020 at 5:44 AM Simon Glass <[hidden email]> wrote:

>
> The extra ACPI code increases U-Boot above it current size limit. Move
> the start earlier to provide space.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Add new patch to allow more space for U-Boot on link
>
>  configs/chromebook_link_defconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 40/57] x86: fsp: Update the FSP API with the end-firmware method

Bin Meng-2
In reply to this post by Simon Glass-3
On Mon, Sep 7, 2020 at 5:44 AM Simon Glass <[hidden email]> wrote:

>
> This new method is intended to be called when UEFI shuts down the 'boot
> services', i.e. any lingering code in the boot loader that might be used
> by the OS.
>
> Add a definition for this new method and update the comments a little.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v1)
>
>  arch/x86/include/asm/fsp/fsp_api.h | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>

Reviewed-by: Bin Meng <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 44/57] x86: Correct the assembly guard in e820.h

Bin Meng-2
In reply to this post by Simon Glass-3
On Mon, Sep 7, 2020 at 5:44 AM Simon Glass <[hidden email]> wrote:

>
> This is currently in the wrong place, so including the file in the device
> tree fails. Fix it.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v1)
>
> Changes in v1:
> - Update commit message with a comma
>
>  arch/x86/include/asm/e820.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>

Reviewed-by: Bin Meng <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 45/57] x86: Add a header guard to asm/acpi_table.h

Bin Meng-2
In reply to this post by Simon Glass-3
On Mon, Sep 7, 2020 at 5:44 AM Simon Glass <[hidden email]> wrote:

>
> This file cannot currently be included in ASL files. Add a header guard
> to permit this.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v1)
>
>  arch/x86/include/asm/acpi_table.h | 4 ++++
>  1 file changed, 4 insertions(+)
>

Reviewed-by: Bin Meng <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 49/57] x86: acpi: Set the log category for x86 table generation

Bin Meng-2
In reply to this post by Simon Glass-3
On Mon, Sep 7, 2020 at 5:44 AM Simon Glass <[hidden email]> wrote:

>
> This file doesn't currently have a log category. Add one so that items
> are logged correctly.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v1)
>
>  arch/x86/lib/acpi_table.c | 2 ++
>  1 file changed, 2 insertions(+)
>

Reviewed-by: Bin Meng <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 52/57] x86: fsp: Add more debugging for silicon init

Bin Meng-2
In reply to this post by Simon Glass-3
On Mon, Sep 7, 2020 at 5:45 AM Simon Glass <[hidden email]> wrote:

>
> If locating the FSP header hangs for whatever reason it is useful to see
> where it got stuck. Add a debug print. Also show the address of the FSP-S
> entry point as a sanity check.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v1)
>
>  arch/x86/lib/fsp2/fsp_silicon_init.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 53/57] x86: fsp: Show FSP-S or FSP-M address in fsp_get_header()

Bin Meng-2
In reply to this post by Simon Glass-3
On Mon, Sep 7, 2020 at 5:45 AM Simon Glass <[hidden email]> wrote:

>
> At present this function only supports FSP-M but it is also used to read
> FSP-S, in which case FSP-M may be zero. Add support for showing whichever
> address is present in the FSP binary.
>
> Also change the debug() statements to log_debug() while here.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v1)
>
>  arch/x86/lib/fsp2/fsp_support.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>

Reviewed-by: Bin Meng <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 54/57] acpi: Use defines for field lengths

Bin Meng-2
In reply to this post by Simon Glass-3
On Mon, Sep 7, 2020 at 5:45 AM Simon Glass <[hidden email]> wrote:

>
> A few fields have an open-coded length. Use the defines for this purpose
> instead.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v1)
>
>  include/acpi/acpi_table.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>

Reviewed-by: Bin Meng <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 55/57] x86: Add a way to add to the e820 memory table

Bin Meng-2
In reply to this post by Simon Glass-3
On Mon, Sep 7, 2020 at 5:45 AM Simon Glass <[hidden email]> wrote:

>
> Some boards want to reserve extra regions of memory. Add a 'chosen'
> property to support this.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v1)
>
>  arch/x86/lib/fsp/fsp_dram.c         | 17 +++++++++++++++++
>  doc/device-tree-bindings/chosen.txt | 18 ++++++++++++++++++
>  2 files changed, 35 insertions(+)
>

Reviewed-by: Bin Meng <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 56/57] x86: Move include of bitops out of ACPI region

Bin Meng-2
In reply to this post by Simon Glass-3
On Mon, Sep 7, 2020 at 5:45 AM Simon Glass <[hidden email]> wrote:

>
> At present linux/bitops.h is included in ACPI code. This is not needed and
> can cause a problem in fls64.h since BITS_PER_LONG is not defined. Move
> the #include into the part not used by ACPI.
>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Add new patch to move include of bitops out of ACPI region
>
>  include/acpi/acpi_table.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 39/57] tpm: cr50: Add ACPI support

Bin Meng-2
In reply to this post by Andy Shevchenko-2
Hi Simon,

On Mon, Sep 21, 2020 at 7:51 PM Andy Shevchenko
<[hidden email]> wrote:

>
> On Sun, Sep 06, 2020 at 03:43:47PM -0600, Simon Glass wrote:
> > Generate ACPI information for this device so that Linux can use it
> > correctly.
>
> > +     ret = acpi_device_write_interrupt_or_gpio(ctx, (struct udevice *)dev,
> > +                                               "ready-gpios");
> > +     if (ret < 0)
> > +             return log_msg_ret("irq_gpio", ret);
>
> I looked a bit closer at the acpi_table.c and would like to emphasize on
> lessons learn from BIOS mistakes found in the wild with GPIOs.
>
> Because GPIO resources are quite badly described in ACPI (it seems MS failed to
> deliver GPIO abstraction to ACPI and to Windows API), there are some corner
> cases, in order to mitigate which we need to consider the following to avoid
> potential glitches and misconfiguration:
>
> - GpioIo() doesn't have any means of Active Low / High setting, the _DSD must
>   be provided to mitigate this.
>
> - GpioIo() doesn't properly communicate the initial state of the output pin,
>   thus Linux assumes the simple rule:
>
>   Pull Bias       Polarity      Requested...
>
>   Implicit        x             AS IS (assumed firmware configured for us)
>   Explicit        x (no _DSD)   as Pull Bias (Up == High, Down == Low),
>                                 assuming non-active (Polarity = !Pull Bias)
>
>   Down            Low           as low, assuming active
>   Down            High          as high, assuming non-active
>   Up              Low           as low, assuming non-active
>   Up              High          as high, assuming active
>
> Hopefully this helps (and maybe can be added to some documentation).
>
> P.S. Why I2cSerialBus() and not I2cSerialBusV2() ?
>

Will you address Andy's comments by adding documentation in the next version?

Regards,
Bin
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 00/57] dm: Add programatic generation of ACPI tables (part D)

Bin Meng-2
In reply to this post by Simon Glass-3
Hi Simon,

On Mon, Sep 7, 2020 at 5:44 AM Simon Glass <[hidden email]> wrote:

>
> Note: This is part D of this effort. With this, Coral includes all
> required ACPI tables.
>
> At present on x86 U-Boot supports creating ACPI (Advanced Configuration
> and Power Interface) tables using the Intel ACPI Source Language (ASL)
> compiler.
>
> This is good enough for basic operation but some devices need to add
> their information dynamically at runtime. An example is a device that
> needs to report its enable GPIO. This is described in the device tree,
> so we want to add code in the driver to convert that device-tree
> description into an ACPI description for use on Linux.
>
> This series adds support for generation of ACPI tables and fragments by
> devices. The core support is built into driver model.
>
> Several files are brought over from coreboot to do the actual generation.
>
> As an example of using this new feature, chromebook_coral is updated to
> write out a wide array of ACPI tables including DSDT and SSDT.
>
> This initial version of the series lays out the general approach. More
> work is needed to figure out the difference between CONFIG_ACPIGEN and
> CONFIG_GENERATE_ACPI_TABLE with respect to what is built.
>

This series does not apply on u-boot-x86/next. Could you please
rebase, and address Andy's comments?

Regards,
Bin
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 39/57] tpm: cr50: Add ACPI support

Andy Shevchenko
In reply to this post by Andy Shevchenko-2
On Mon, Sep 21, 2020 at 2:51 PM Andy Shevchenko
<[hidden email]> wrote:

>
> On Sun, Sep 06, 2020 at 03:43:47PM -0600, Simon Glass wrote:
> > Generate ACPI information for this device so that Linux can use it
> > correctly.
>
> > +     ret = acpi_device_write_interrupt_or_gpio(ctx, (struct udevice *)dev,
> > +                                               "ready-gpios");
> > +     if (ret < 0)
> > +             return log_msg_ret("irq_gpio", ret);
>
> I looked a bit closer at the acpi_table.c and would like to emphasize on
> lessons learn from BIOS mistakes found in the wild with GPIOs.
>
> Because GPIO resources are quite badly described in ACPI (it seems MS failed to
> deliver GPIO abstraction to ACPI and to Windows API), there are some corner
> cases, in order to mitigate which we need to consider the following to avoid
> potential glitches and misconfiguration:
>
> - GpioIo() doesn't have any means of Active Low / High setting, the _DSD must
>   be provided to mitigate this.
>
> - GpioIo() doesn't properly communicate the initial state of the output pin,
>   thus Linux assumes the simple rule:
>
>   Pull Bias       Polarity      Requested...
>
>   Implicit        x             AS IS (assumed firmware configured for us)
>   Explicit        x (no _DSD)   as Pull Bias (Up == High, Down == Low),
>                                 assuming non-active (Polarity = !Pull Bias)


>   Down            Low           as low, assuming active
>   Down            High          as high, assuming non-active
>   Up              Low           as low, assuming non-active
>   Up              High          as high, assuming active

I re-read the above piece of the table and found that I mistakenly
placed words after 'as ' part.
Should be
 as low, ...
 as low, ...
 as high, ...
 as high, ...

So, request follows the bias setting, but polarity, if explicitly
present, defines active/non-active state.

> Hopefully this helps (and maybe can be added to some documentation).
>
> P.S. Why I2cSerialBus() and not I2cSerialBusV2() ?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>


--
With Best Regards,
Andy Shevchenko
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices

Simon Glass-3
In reply to this post by Wolfgang Wallner
Hi Wolfgang,

On Mon, 21 Sep 2020 at 07:50, Wolfgang Wallner
<[hidden email]> wrote:

>
> Hi Simon,
>
> -----"Simon Glass" <[hidden email]> schrieb: -----
> > Betreff: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
> >
> > Add common x86 ASL files, taken from coreboot.
> >
> > Signed-off-by: Simon Glass <[hidden email]>
> > ---
> >
> > (no changes since v1)
> >
> >  arch/x86/include/asm/acpi/chromeos.asl    | 108 +++++++++++++++++
> >  arch/x86/include/asm/acpi/cpu.asl         |  25 ++++
> >  arch/x86/include/asm/acpi/cros_gnvs.asl   |  29 +++++
> >  arch/x86/include/asm/acpi/lpc.asl         | 141 ++++++++++++++++++++++
> >  arch/x86/include/asm/acpi/pci_osc.asl     |  21 ++++
> >  arch/x86/include/asm/acpi/pcr.asl         |  80 ++++++++++++
> >  arch/x86/include/asm/acpi/ramoops.asl     |  32 +++++
> >  arch/x86/include/asm/acpi/sleepstates.asl |  12 +-
> >  8 files changed, 443 insertions(+), 5 deletions(-)
> >  create mode 100644 arch/x86/include/asm/acpi/chromeos.asl
> >  create mode 100644 arch/x86/include/asm/acpi/cpu.asl
> >  create mode 100644 arch/x86/include/asm/acpi/cros_gnvs.asl
> >  create mode 100644 arch/x86/include/asm/acpi/lpc.asl
> >  create mode 100644 arch/x86/include/asm/acpi/pci_osc.asl
> >  create mode 100644 arch/x86/include/asm/acpi/pcr.asl
> >  create mode 100644 arch/x86/include/asm/acpi/ramoops.asl
>
> There are recent (~June 2020) commits in coreboot which convert most ASL files
> to ASL 2.0. The ASL 2.0 syntax is much easier to read IMHO, and I think it
> would be good to update this patch to take advantage of the recent coreboot
> developments. See e.g. the commit at [1].

Yes that looks nicer, but I have everything working and tested now so
don't want to go back and unpick things. This seems like something we
can pick up once we finally have this in the tree. The good thing is
that this is just a syntax change as I understand it.

A WFH project has been putting a few Chromebooks in my lab, and I
expect to put coral in there once everything lands. That should give
us the confidence to make minor and major changes in future.

Regards,
SImon

> [1] https://github.com/coreboot/coreboot/commit/03248033e7be6f81ad5b60ed21a60071aee32c67
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices

Wolfgang Wallner
Hi Simon,

-----"Simon Glass" <[hidden email]> schrieb: -----

> Betreff: Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
>
> Hi Wolfgang,
>
> On Mon, 21 Sep 2020 at 07:50, Wolfgang Wallner
> <[hidden email]> wrote:
> >
> > Hi Simon,
> >
> > -----"Simon Glass" <[hidden email]> schrieb: -----
> > > Betreff: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
> > >
> > > Add common x86 ASL files, taken from coreboot.
> > >
> > > Signed-off-by: Simon Glass <[hidden email]>
> > > ---
> > >
> > > (no changes since v1)
> > >
> > >  arch/x86/include/asm/acpi/chromeos.asl    | 108 +++++++++++++++++
> > >  arch/x86/include/asm/acpi/cpu.asl         |  25 ++++
> > >  arch/x86/include/asm/acpi/cros_gnvs.asl   |  29 +++++
> > >  arch/x86/include/asm/acpi/lpc.asl         | 141 ++++++++++++++++++++++
> > >  arch/x86/include/asm/acpi/pci_osc.asl     |  21 ++++
> > >  arch/x86/include/asm/acpi/pcr.asl         |  80 ++++++++++++
> > >  arch/x86/include/asm/acpi/ramoops.asl     |  32 +++++
> > >  arch/x86/include/asm/acpi/sleepstates.asl |  12 +-
> > >  8 files changed, 443 insertions(+), 5 deletions(-)
> > >  create mode 100644 arch/x86/include/asm/acpi/chromeos.asl
> > >  create mode 100644 arch/x86/include/asm/acpi/cpu.asl
> > >  create mode 100644 arch/x86/include/asm/acpi/cros_gnvs.asl
> > >  create mode 100644 arch/x86/include/asm/acpi/lpc.asl
> > >  create mode 100644 arch/x86/include/asm/acpi/pci_osc.asl
> > >  create mode 100644 arch/x86/include/asm/acpi/pcr.asl
> > >  create mode 100644 arch/x86/include/asm/acpi/ramoops.asl
> >
> > There are recent (~June 2020) commits in coreboot which convert most ASL files
> > to ASL 2.0. The ASL 2.0 syntax is much easier to read IMHO, and I think it
> > would be good to update this patch to take advantage of the recent coreboot
> > developments. See e.g. the commit at [1].
>
> Yes that looks nicer, but I have everything working and tested now so
> don't want to go back and unpick things. This seems like something we
> can pick up once we finally have this in the tree. The good thing is
> that this is just a syntax change as I understand it.

Fair enough. Yes, it should only be a syntax change.

Regardingn testing: I haven't tested individual patches of part D, but I have
tested the complete series. After applying the fix for the DSDT length my
ApolloLake board has booted reliably with ACPI enabled.

I have some remaining issues that I haven't gone after yet (e.g. I see a
machine check error in the log), but the board always boots.

> A WFH project has been putting a few Chromebooks in my lab, and I
> expect to put coral in there once everything lands. That should give
> us the confidence to make minor and major changes in future.
>
> Regards,
> SImon
>
> > [1] https://github.com/coreboot/coreboot/commit/03248033e7be6f81ad5b60ed21a60071aee32c67
> >

regards, Wolfgang

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices

Bin Meng-2
Hi Wolfgang,

On Tue, Sep 22, 2020 at 10:19 PM Wolfgang Wallner
<[hidden email]> wrote:

>
> Hi Simon,
>
> -----"Simon Glass" <[hidden email]> schrieb: -----
> > Betreff: Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
> >
> > Hi Wolfgang,
> >
> > On Mon, 21 Sep 2020 at 07:50, Wolfgang Wallner
> > <[hidden email]> wrote:
> > >
> > > Hi Simon,
> > >
> > > -----"Simon Glass" <[hidden email]> schrieb: -----
> > > > Betreff: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
> > > >
> > > > Add common x86 ASL files, taken from coreboot.
> > > >
> > > > Signed-off-by: Simon Glass <[hidden email]>
> > > > ---
> > > >
> > > > (no changes since v1)
> > > >
> > > >  arch/x86/include/asm/acpi/chromeos.asl    | 108 +++++++++++++++++
> > > >  arch/x86/include/asm/acpi/cpu.asl         |  25 ++++
> > > >  arch/x86/include/asm/acpi/cros_gnvs.asl   |  29 +++++
> > > >  arch/x86/include/asm/acpi/lpc.asl         | 141 ++++++++++++++++++++++
> > > >  arch/x86/include/asm/acpi/pci_osc.asl     |  21 ++++
> > > >  arch/x86/include/asm/acpi/pcr.asl         |  80 ++++++++++++
> > > >  arch/x86/include/asm/acpi/ramoops.asl     |  32 +++++
> > > >  arch/x86/include/asm/acpi/sleepstates.asl |  12 +-
> > > >  8 files changed, 443 insertions(+), 5 deletions(-)
> > > >  create mode 100644 arch/x86/include/asm/acpi/chromeos.asl
> > > >  create mode 100644 arch/x86/include/asm/acpi/cpu.asl
> > > >  create mode 100644 arch/x86/include/asm/acpi/cros_gnvs.asl
> > > >  create mode 100644 arch/x86/include/asm/acpi/lpc.asl
> > > >  create mode 100644 arch/x86/include/asm/acpi/pci_osc.asl
> > > >  create mode 100644 arch/x86/include/asm/acpi/pcr.asl
> > > >  create mode 100644 arch/x86/include/asm/acpi/ramoops.asl
> > >
> > > There are recent (~June 2020) commits in coreboot which convert most ASL files
> > > to ASL 2.0. The ASL 2.0 syntax is much easier to read IMHO, and I think it
> > > would be good to update this patch to take advantage of the recent coreboot
> > > developments. See e.g. the commit at [1].
> >
> > Yes that looks nicer, but I have everything working and tested now so
> > don't want to go back and unpick things. This seems like something we
> > can pick up once we finally have this in the tree. The good thing is
> > that this is just a syntax change as I understand it.
>
> Fair enough. Yes, it should only be a syntax change.
>
> Regardingn testing: I haven't tested individual patches of part D, but I have
> tested the complete series. After applying the fix for the DSDT length my
> ApolloLake board has booted reliably with ACPI enabled.
>

Will you add the "Tested-by" tag for this series?

BTW: any plan to upstream your board support?

> I have some remaining issues that I haven't gone after yet (e.g. I see a
> machine check error in the log), but the board always boots.
>

Regards,
Bin
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices

Wolfgang Wallner
Hi Bin,

-----"Bin Meng" <[hidden email]> schrieb: -----

> Betreff: Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
>
> Hi Wolfgang,
>
> On Tue, Sep 22, 2020 at 10:19 PM Wolfgang Wallner
> <[hidden email]> wrote:
> >
> > Hi Simon,
> >
> > -----"Simon Glass" <[hidden email]> schrieb: -----
> > > Betreff: Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
> > >
> > > Hi Wolfgang,
> > >
> > > On Mon, 21 Sep 2020 at 07:50, Wolfgang Wallner
> > > <[hidden email]> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > -----"Simon Glass" <[hidden email]> schrieb: -----
> > > > > Betreff: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
> > > > >
> > > > > Add common x86 ASL files, taken from coreboot.
> > > > >
> > > > > Signed-off-by: Simon Glass <[hidden email]>
> > > > > ---
> > > > >
> > > > > (no changes since v1)
> > > > >
> > > > >  arch/x86/include/asm/acpi/chromeos.asl    | 108 +++++++++++++++++
> > > > >  arch/x86/include/asm/acpi/cpu.asl         |  25 ++++
> > > > >  arch/x86/include/asm/acpi/cros_gnvs.asl   |  29 +++++
> > > > >  arch/x86/include/asm/acpi/lpc.asl         | 141 ++++++++++++++++++++++
> > > > >  arch/x86/include/asm/acpi/pci_osc.asl     |  21 ++++
> > > > >  arch/x86/include/asm/acpi/pcr.asl         |  80 ++++++++++++
> > > > >  arch/x86/include/asm/acpi/ramoops.asl     |  32 +++++
> > > > >  arch/x86/include/asm/acpi/sleepstates.asl |  12 +-
> > > > >  8 files changed, 443 insertions(+), 5 deletions(-)
> > > > >  create mode 100644 arch/x86/include/asm/acpi/chromeos.asl
> > > > >  create mode 100644 arch/x86/include/asm/acpi/cpu.asl
> > > > >  create mode 100644 arch/x86/include/asm/acpi/cros_gnvs.asl
> > > > >  create mode 100644 arch/x86/include/asm/acpi/lpc.asl
> > > > >  create mode 100644 arch/x86/include/asm/acpi/pci_osc.asl
> > > > >  create mode 100644 arch/x86/include/asm/acpi/pcr.asl
> > > > >  create mode 100644 arch/x86/include/asm/acpi/ramoops.asl
> > > >
> > > > There are recent (~June 2020) commits in coreboot which convert most ASL files
> > > > to ASL 2.0. The ASL 2.0 syntax is much easier to read IMHO, and I think it
> > > > would be good to update this patch to take advantage of the recent coreboot
> > > > developments. See e.g. the commit at [1].
> > >
> > > Yes that looks nicer, but I have everything working and tested now so
> > > don't want to go back and unpick things. This seems like something we
> > > can pick up once we finally have this in the tree. The good thing is
> > > that this is just a syntax change as I understand it.
> >
> > Fair enough. Yes, it should only be a syntax change.
> >
> > Regardingn testing: I haven't tested individual patches of part D, but I have
> > tested the complete series. After applying the fix for the DSDT length my
> > ApolloLake board has booted reliably with ACPI enabled.
> >
>
> Will you add the "Tested-by" tag for this series?

I lack experience in testing complete series, which is why I was not sure how
this should be handled. If testing a complete series at once is enough, without
testing the individual patches, then this series is:

Tested-by: Wolfgang Wallner <[hidden email]>
Tested on a custom ApolloLake board.

> BTW: any plan to upstream your board support?

Yes, that would be one of the long term goals.

regards, Wolfgang

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices

Bin Meng-2
Hi Wolfgang,

On Tue, Sep 22, 2020 at 10:47 PM Wolfgang Wallner
<[hidden email]> wrote:

>
> Hi Bin,
>
> -----"Bin Meng" <[hidden email]> schrieb: -----
> > Betreff: Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
> >
> > Hi Wolfgang,
> >
> > On Tue, Sep 22, 2020 at 10:19 PM Wolfgang Wallner
> > <[hidden email]> wrote:
> > >
> > > Hi Simon,
> > >
> > > -----"Simon Glass" <[hidden email]> schrieb: -----
> > > > Betreff: Re: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
> > > >
> > > > Hi Wolfgang,
> > > >
> > > > On Mon, 21 Sep 2020 at 07:50, Wolfgang Wallner
> > > > <[hidden email]> wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > -----"Simon Glass" <[hidden email]> schrieb: -----
> > > > > > Betreff: [PATCH v3 02/57] x86: acpi: Add base asl files for common x86 devices
> > > > > >
> > > > > > Add common x86 ASL files, taken from coreboot.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <[hidden email]>
> > > > > > ---
> > > > > >
> > > > > > (no changes since v1)
> > > > > >
> > > > > >  arch/x86/include/asm/acpi/chromeos.asl    | 108 +++++++++++++++++
> > > > > >  arch/x86/include/asm/acpi/cpu.asl         |  25 ++++
> > > > > >  arch/x86/include/asm/acpi/cros_gnvs.asl   |  29 +++++
> > > > > >  arch/x86/include/asm/acpi/lpc.asl         | 141 ++++++++++++++++++++++
> > > > > >  arch/x86/include/asm/acpi/pci_osc.asl     |  21 ++++
> > > > > >  arch/x86/include/asm/acpi/pcr.asl         |  80 ++++++++++++
> > > > > >  arch/x86/include/asm/acpi/ramoops.asl     |  32 +++++
> > > > > >  arch/x86/include/asm/acpi/sleepstates.asl |  12 +-
> > > > > >  8 files changed, 443 insertions(+), 5 deletions(-)
> > > > > >  create mode 100644 arch/x86/include/asm/acpi/chromeos.asl
> > > > > >  create mode 100644 arch/x86/include/asm/acpi/cpu.asl
> > > > > >  create mode 100644 arch/x86/include/asm/acpi/cros_gnvs.asl
> > > > > >  create mode 100644 arch/x86/include/asm/acpi/lpc.asl
> > > > > >  create mode 100644 arch/x86/include/asm/acpi/pci_osc.asl
> > > > > >  create mode 100644 arch/x86/include/asm/acpi/pcr.asl
> > > > > >  create mode 100644 arch/x86/include/asm/acpi/ramoops.asl
> > > > >
> > > > > There are recent (~June 2020) commits in coreboot which convert most ASL files
> > > > > to ASL 2.0. The ASL 2.0 syntax is much easier to read IMHO, and I think it
> > > > > would be good to update this patch to take advantage of the recent coreboot
> > > > > developments. See e.g. the commit at [1].
> > > >
> > > > Yes that looks nicer, but I have everything working and tested now so
> > > > don't want to go back and unpick things. This seems like something we
> > > > can pick up once we finally have this in the tree. The good thing is
> > > > that this is just a syntax change as I understand it.
> > >
> > > Fair enough. Yes, it should only be a syntax change.
> > >
> > > Regardingn testing: I haven't tested individual patches of part D, but I have
> > > tested the complete series. After applying the fix for the DSDT length my
> > > ApolloLake board has booted reliably with ACPI enabled.
> > >
> >
> > Will you add the "Tested-by" tag for this series?
>
> I lack experience in testing complete series, which is why I was not sure how
> this should be handled. If testing a complete series at once is enough, without
> testing the individual patches, then this series is:
>
> Tested-by: Wolfgang Wallner <[hidden email]>
> Tested on a custom ApolloLake board.

Thank you. But I will leave Simon to pick up the tag in the next
version whenever it makes sense.

>
> > BTW: any plan to upstream your board support?
>
> Yes, that would be one of the long term goals.

Great!

Regards,
Bin
1234