From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Wallner Date: Wed, 8 Jul 2020 13:05:50 +0200 Subject: [PATCH v1 22/43] x86: Add support for building up an NHLT structure In-Reply-To: References: Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, -----"Simon Glass" schrieb: ----- > Betreff: Re: [PATCH v1 22/43] x86: Add support for building up an NHLT structure > > Hi Wolfgang, > > On Thu, 2 Jul 2020 at 02:11, Wolfgang Wallner > wrote: > > > > Hi Simon, > > > > I dont know NHLT well enough to actually review the code, but I did compare > > the files in this patch to the version in coreboot. Most of the changes are > > obvious (coding style, spelling, ...), but some things are not, see the remarks > > below. > > > > -----"Simon Glass" schrieb: ----- > > > Betreff: [PATCH v1 22/43] x86: Add support for building up an NHLT structure > > > > > > The Intel Non-High-Definition-Audio Link Table (NHLT) table describes the > > > audio codecs and connections in a system. Various devices can contribute > > > information to produce the table. > > > > > > Add functions to allow adding to the structure that is eventually written > > > to the ACPI tables. Also add the device-tree bindings. > > > > > > Signed-off-by: Simon Glass > > > --- > > > > > > Changes in v1: > > > - Add a new patch to support building up an NHLT structure > > > > > > arch/x86/include/asm/acpi_nhlt.h | 314 ++++++++++++++++++++ > > > arch/x86/lib/Makefile | 1 + > > > arch/x86/lib/acpi_nhlt.c | 482 +++++++++++++++++++++++++++++++ > > > 3 files changed, 797 insertions(+) > > > create mode 100644 arch/x86/include/asm/acpi_nhlt.h > > > create mode 100644 arch/x86/lib/acpi_nhlt.c > > > > > > diff --git a/arch/x86/include/asm/acpi_nhlt.h b/arch/x86/include/asm/acpi_nhlt.h > > > new file mode 100644 > > > index 0000000000..4d2573d5ff > > > --- /dev/null > > > +++ b/arch/x86/include/asm/acpi_nhlt.h [snip] > > > + > > > +/* > > > + * Serialize NHLT object to ACPI table. Take in the beginning address of where > > > + * the table will reside and return the address of the next ACPI table. On > > > + * error 0 will be returned. The NHLT object is no longer valid after this > > > + * function is called. > > > + */ > > > +uintptr_t nhlt_serialise(struct nhlt *nhlt, uintptr_t acpi_addr); > > > > The implementation for this functions seems to be missing in this patch. > > In coreboot it looks like this: > > > > uintptr_t nhlt_serialize(struct nhlt *nhlt, uintptr_t acpi_addr) > > { > > return nhlt_serialize_oem_overrides(nhlt, acpi_addr, NULL, NULL, 0); > > } > > Yes we don't need this at present. If we do we would likely put this > in the device tree. Since coreboot doesn't have a proper device tree > it uses code to call override functions to get the data, which IMO > makes it quite hard to work out the config for a particular board If we don't need the implementation of nhlt_serialise(), shouldn't we then also drop its declaration in the header file? > > [..] > regards, Wolfgang