Hi Bin, On Mon, 13 Jul 2020 at 23:01, Bin Meng <bmeng...@gmail.com> wrote: > > Hi Simon, > > On Tue, Jul 14, 2020 at 9:28 AM Bin Meng <bmeng...@gmail.com> wrote: > > > > Hi Simon, > > > > On Wed, Jul 8, 2020 at 11:33 AM Simon Glass <s...@chromium.org> wrote: > > > > > > This chip is used on coral and we need to generate ACPI tables for sound > > > to make it work. Add a driver that does just this (i.e. at present does > > > not actually support playing sound). > > > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > > --- > > > > > > Changes in v2: > > > Add a comment about only x86 boards supporting NHLT > > > > > > Changes in v1: > > > - Use acpi,ddn instead of acpi,desc > > > - Drop the unwanted acpi_device_write_gpio_desc() > > > - Rename max97357a to max98357a > > > - Add NHLT support > > > - Capitalise ACPI_OPS_PTR > > > - Rebase to master > > > > > > configs/sandbox_defconfig | 1 + > > > doc/device-tree-bindings/sound/max98357a.txt | 22 +++ > > > drivers/sound/Kconfig | 9 ++ > > > drivers/sound/Makefile | 1 + > > > drivers/sound/max98357a.c | 161 +++++++++++++++++++ > > > 5 files changed, 194 insertions(+) > > > create mode 100644 doc/device-tree-bindings/sound/max98357a.txt > > > create mode 100644 drivers/sound/max98357a.c > > > > > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig > > > index 3817091d02..0c9524ff6c 100644 > > > --- a/configs/sandbox_defconfig > > > +++ b/configs/sandbox_defconfig > > > @@ -207,6 +207,7 @@ CONFIG_SMEM=y > > > CONFIG_SANDBOX_SMEM=y > > > CONFIG_SOUND=y > > > CONFIG_SOUND_DA7219=y > > > +CONFIG_SOUND_MAX98357A=y > > > CONFIG_SOUND_SANDBOX=y > > > CONFIG_SANDBOX_SPI=y > > > CONFIG_SPMI=y > > > diff --git a/doc/device-tree-bindings/sound/max98357a.txt > > > b/doc/device-tree-bindings/sound/max98357a.txt > > > new file mode 100644 > > > index 0000000000..4bce14ce80 > > > --- /dev/null > > > +++ b/doc/device-tree-bindings/sound/max98357a.txt > > > @@ -0,0 +1,22 @@ > > > +Maxim MAX98357A audio DAC > > > + > > > +This node models the Maxim MAX98357A DAC. > > > + > > > +Required properties: > > > +- compatible : "maxim,max98357a" > > > + > > > +Optional properties: > > > +- sdmode-gpios : GPIO specifier for the chip's SD_MODE pin. > > > + If this option is not specified then driver does not manage > > > + the pin state (e.g. chip is always on). > > > +- sdmode-delay : specify delay time for SD_MODE pin. > > > + If this option is specified, which means it's required i2s clocks > > > + ready before SD_MODE is unmuted in order to avoid the speaker > > > pop noise. > > > + It's observed that 5ms is sufficient. > > > + > > > +Example: > > > + > > > +max98357a { > > > + compatible = "maxim,max98357a"; > > > + sdmode-gpios = <&qcom_pinmux 25 0>; > > > +}; > > > diff --git a/drivers/sound/Kconfig b/drivers/sound/Kconfig > > > index 7f214b97be..0948d8caab 100644 > > > --- a/drivers/sound/Kconfig > > > +++ b/drivers/sound/Kconfig > > > @@ -113,6 +113,15 @@ config SOUND_MAX98095 > > > audio data and I2C for codec control. At present it only works > > > with the Samsung I2S driver. > > > > > > +config SOUND_MAX98357A > > > + bool "Support Maxim max98357a audio codec" > > > + depends on PCI > > > + help > > > + Enable the max98357a audio codec. This is connected on PCI for > > > + audio data codec control. This is currently only capable of > > > providing > > > + ACPI information. A full driver (with sound in U-Boot) is > > > currently > > > + not available. > > > + > > > config SOUND_RT5677 > > > bool "Support Realtek RT5677 audio codec" > > > depends on SOUND > > > diff --git a/drivers/sound/Makefile b/drivers/sound/Makefile > > > index 8c3933ad15..9b40c8012f 100644 > > > --- a/drivers/sound/Makefile > > > +++ b/drivers/sound/Makefile > > > @@ -17,6 +17,7 @@ obj-$(CONFIG_SOUND_WM8994) += wm8994.o > > > obj-$(CONFIG_SOUND_MAX98088) += max98088.o maxim_codec.o > > > obj-$(CONFIG_SOUND_MAX98090) += max98090.o maxim_codec.o > > > obj-$(CONFIG_SOUND_MAX98095) += max98095.o maxim_codec.o > > > +obj-$(CONFIG_SOUND_MAX98357A) += max98357a.o > > > obj-$(CONFIG_SOUND_INTEL_HDA) += hda_codec.o > > > obj-$(CONFIG_SOUND_I8254) += i8254_beep.o > > > obj-$(CONFIG_SOUND_RT5677) += rt5677.o > > > diff --git a/drivers/sound/max98357a.c b/drivers/sound/max98357a.c > > > new file mode 100644 > > > index 0000000000..827262d235 > > > --- /dev/null > > > +++ b/drivers/sound/max98357a.c > > > @@ -0,0 +1,161 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * max98357a.c -- MAX98357A Audio driver > > > + * > > > + * Copyright 2019 Google LLC > > > + * Parts taken from coreboot > > > + */ > > > + > > > +#include <common.h> > > > +#include <audio_codec.h> > > > +#include <dm.h> > > > +#include <log.h> > > > +#include <sound.h> > > > +#include <acpi/acpigen.h> > > > +#include <acpi/acpi_device.h> > > > +#include <acpi/acpi_dp.h> > > > +#include <asm-generic/gpio.h> > > > +#ifdef CONFIG_X86 > > > +#include <asm/acpi_nhlt.h> > > > +#endif > > > +#include <dt-bindings/sound/nhlt.h> > > > +#include <dm/acpi.h> > > > + > > > +struct max98357a_priv { > > > + struct gpio_desc sdmode_gpio; > > > +}; > > > + > > > +static int max98357a_ofdata_to_platdata(struct udevice *dev) > > > +{ > > > + struct max98357a_priv *priv = dev_get_priv(dev); > > > + int ret; > > > + > > > + ret = gpio_request_by_name(dev, "sdmode-gpios", 0, > > > &priv->sdmode_gpio, > > > + GPIOD_IS_IN); > > > + if (ret) > > > + return log_msg_ret("gpio", ret); > > > + > > > + return 0; > > > +} > > > + > > > +static int max98357a_acpi_fill_ssdt(const struct udevice *dev, > > > + struct acpi_ctx *ctx) > > > +{ > > > + struct max98357a_priv *priv = dev_get_priv(dev); > > > + char scope[ACPI_PATH_MAX]; > > > + char name[ACPI_NAME_MAX]; > > > + char path[ACPI_PATH_MAX]; > > > + struct acpi_dp *dp; > > > + int ret; > > > + > > > + ret = acpi_device_scope(dev, scope, sizeof(scope)); > > > + if (ret) > > > + return log_msg_ret("scope", ret); > > > + ret = acpi_get_name(dev, name); > > > + if (ret) > > > + return log_msg_ret("name", ret); > > > + > > > + /* Device */ > > > + acpigen_write_scope(ctx, scope); > > > + acpigen_write_device(ctx, name); > > > + acpigen_write_name_string(ctx, "_HID", > > > + dev_read_string(dev, "acpi,hid")); > > > + acpigen_write_name_integer(ctx, "_UID", 0); > > > + acpigen_write_name_string(ctx, "_DDN", > > > + dev_read_string(dev, "acpi,ddn")); > > > + acpigen_write_sta(ctx, acpi_device_status(dev)); > > > + > > > + /* Resources */ > > > + acpigen_write_name(ctx, "_CRS"); > > > + acpigen_write_resourcetemplate_header(ctx); > > > + ret = acpi_device_write_gpio_desc(ctx, &priv->sdmode_gpio); > > > + if (ret) > > > + return log_msg_ret("gpio", ret); > > > + acpigen_write_resourcetemplate_footer(ctx); > > > + > > > + /* _DSD for devicetree properties */ > > > + /* This points to the first pin in the first gpio entry in _CRS */ > > > + ret = acpi_device_path(dev, path, sizeof(path)); > > > + if (ret) > > > + return log_msg_ret("path", ret); > > > + dp = acpi_dp_new_table("_DSD"); > > > + acpi_dp_add_gpio(dp, "sdmode-gpio", path, 0, 0, > > > + priv->sdmode_gpio.flags & GPIOD_ACTIVE_LOW ? > > > + ACPI_GPIO_ACTIVE_LOW : ACPI_GPIO_ACTIVE_HIGH); > > > > There is a bug here: > > > > It should be: ACPI_IRQ_ACTIVE_LOW : ACPI_IRQ_ACTIVE_HIGH > > > > I also noticed that ACPI_IRQ_ACTIVE_LOW value is 0 but > > ACPI_GPIO_ACTIVE_LOW value is 1. Is this intentional? > > > > Please suggest how to fix this, so I can apply the fix locally. > > Currently this is the only failure that was reported by CI pipelines. > > Fixed this by: > https://gitlab.denx.de/u-boot/custodians/u-boot-x86/-/commit/43b45b056237d498adc18425c9e0322612d29596#9eb33518771f2169e0ada514f05b74ea8f65d082_0_84 > > CI all pass now: > https://dev.azure.com/bmeng/GitHub/_build/results?buildId=260&view=results
OK great, thanks! So just the MTRR series to sort out now/? I also have the final ACPI series in progress. I'd love to get it all done this cycle if possible. - Simon