diff mbox series

[v2,17/44] sound: Add an ACPI driver for Maxim MAX98357ac

Message ID 20200707213233.v2.17.Ia2e108d28eecf6576fd92294b9871cd94143a01f@changeid
State Superseded
Headers show
Series x86: Programmatic generation of ACPI tables (Part C) | expand

Commit Message

Simon Glass July 8, 2020, 3:32 a.m. UTC
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 <sjg at 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

Comments

Bin Meng July 14, 2020, 1:28 a.m. UTC | #1
Hi Simon,

On Wed, Jul 8, 2020 at 11:33 AM Simon Glass <sjg at 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 <sjg at 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.

> +       acpi_dp_add_integer(dp, "sdmode-delay",
> +                           dev_read_u32_default(dev, "sdmode-delay", 0));
> +       acpi_dp_write(ctx, dp);
> +
> +       acpigen_pop_len(ctx); /* Device */
> +       acpigen_pop_len(ctx); /* Scope */
> +
> +       return 0;
> +}
> +
> +/* For now only X86 boards support NHLT */
> +#ifdef CONFIG_X86
> +static const struct nhlt_format_config max98357a_formats[] = {
> +       /* 48 KHz 24-bits per sample. */
> +       {
> +               .num_channels = 2,
> +               .sample_freq_khz = 48,
> +               .container_bits_per_sample = 32,
> +               .valid_bits_per_sample = 24,
> +               .settings_file = "max98357-render-2ch-48khz-24b.dat",
> +       },
> +};
> +
> +static const struct nhlt_endp_descriptor max98357a_descriptors[] = {
> +       {
> +               .link = NHLT_LINK_SSP,
> +               .device = NHLT_SSP_DEV_I2S,
> +               .direction = NHLT_DIR_RENDER,
> +               .vid = NHLT_VID,
> +               .did = NHLT_DID_SSP,
> +               .formats = max98357a_formats,
> +               .num_formats = ARRAY_SIZE(max98357a_formats),
> +       },
> +};
> +
> +static int max98357a_acpi_setup_nhlt(const struct udevice *dev,
> +                                    struct acpi_ctx *ctx)
> +{
> +       u32 hwlink;
> +       int ret;
> +
> +       if (dev_read_u32(dev, "acpi,audio-link", &hwlink))
> +               return log_msg_ret("link", -EINVAL);
> +
> +       /* Virtual bus id of SSP links are the hardware port ids proper. */
> +       ret = nhlt_add_ssp_endpoints(ctx->nhlt, hwlink, max98357a_descriptors,
> +                                    ARRAY_SIZE(max98357a_descriptors));
> +       if (ret)
> +               return log_msg_ret("add", ret);
> +
> +       return 0;
> +}
> +#endif
> +
> +struct acpi_ops max98357a_acpi_ops = {
> +       .fill_ssdt      = max98357a_acpi_fill_ssdt,
> +#ifdef CONFIG_X86
> +       .setup_nhlt     = max98357a_acpi_setup_nhlt,
> +#endif
> +};
> +
> +static const struct audio_codec_ops max98357a_ops = {
> +};
> +
> +static const struct udevice_id max98357a_ids[] = {
> +       { .compatible = "maxim,max98357a" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(max98357a) = {
> +       .name           = "max98357a",
> +       .id             = UCLASS_AUDIO_CODEC,
> +       .of_match       = max98357a_ids,
> +       .ofdata_to_platdata     = max98357a_ofdata_to_platdata,
> +       .ops            = &max98357a_ops,
> +       ACPI_OPS_PTR(&max98357a_acpi_ops)
> +};
> --

Regards,
Bin
Bin Meng July 14, 2020, 5:01 a.m. UTC | #2
Hi Simon,

On Tue, Jul 14, 2020 at 9:28 AM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Wed, Jul 8, 2020 at 11:33 AM Simon Glass <sjg at 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 <sjg at 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

>
> > +       acpi_dp_add_integer(dp, "sdmode-delay",
> > +                           dev_read_u32_default(dev, "sdmode-delay", 0));
> > +       acpi_dp_write(ctx, dp);
> > +
> > +       acpigen_pop_len(ctx); /* Device */
> > +       acpigen_pop_len(ctx); /* Scope */
> > +
> > +       return 0;
> > +}
> > +
> > +/* For now only X86 boards support NHLT */
> > +#ifdef CONFIG_X86
> > +static const struct nhlt_format_config max98357a_formats[] = {
> > +       /* 48 KHz 24-bits per sample. */
> > +       {
> > +               .num_channels = 2,
> > +               .sample_freq_khz = 48,
> > +               .container_bits_per_sample = 32,
> > +               .valid_bits_per_sample = 24,
> > +               .settings_file = "max98357-render-2ch-48khz-24b.dat",
> > +       },
> > +};
> > +
> > +static const struct nhlt_endp_descriptor max98357a_descriptors[] = {
> > +       {
> > +               .link = NHLT_LINK_SSP,
> > +               .device = NHLT_SSP_DEV_I2S,
> > +               .direction = NHLT_DIR_RENDER,
> > +               .vid = NHLT_VID,
> > +               .did = NHLT_DID_SSP,
> > +               .formats = max98357a_formats,
> > +               .num_formats = ARRAY_SIZE(max98357a_formats),
> > +       },
> > +};
> > +
> > +static int max98357a_acpi_setup_nhlt(const struct udevice *dev,
> > +                                    struct acpi_ctx *ctx)
> > +{
> > +       u32 hwlink;
> > +       int ret;
> > +
> > +       if (dev_read_u32(dev, "acpi,audio-link", &hwlink))
> > +               return log_msg_ret("link", -EINVAL);
> > +
> > +       /* Virtual bus id of SSP links are the hardware port ids proper. */
> > +       ret = nhlt_add_ssp_endpoints(ctx->nhlt, hwlink, max98357a_descriptors,
> > +                                    ARRAY_SIZE(max98357a_descriptors));
> > +       if (ret)
> > +               return log_msg_ret("add", ret);
> > +
> > +       return 0;
> > +}
> > +#endif
> > +
> > +struct acpi_ops max98357a_acpi_ops = {
> > +       .fill_ssdt      = max98357a_acpi_fill_ssdt,
> > +#ifdef CONFIG_X86
> > +       .setup_nhlt     = max98357a_acpi_setup_nhlt,
> > +#endif
> > +};
> > +
> > +static const struct audio_codec_ops max98357a_ops = {
> > +};
> > +
> > +static const struct udevice_id max98357a_ids[] = {
> > +       { .compatible = "maxim,max98357a" },
> > +       { }
> > +};
> > +
> > +U_BOOT_DRIVER(max98357a) = {
> > +       .name           = "max98357a",
> > +       .id             = UCLASS_AUDIO_CODEC,
> > +       .of_match       = max98357a_ids,
> > +       .ofdata_to_platdata     = max98357a_ofdata_to_platdata,
> > +       .ops            = &max98357a_ops,
> > +       ACPI_OPS_PTR(&max98357a_acpi_ops)
> > +};
> > --

Regards,
Bin
Simon Glass July 14, 2020, 1:25 p.m. UTC | #3
Hi Bin,

On Mon, 13 Jul 2020 at 23:01, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Jul 14, 2020 at 9:28 AM Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Wed, Jul 8, 2020 at 11:33 AM Simon Glass <sjg at 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 <sjg at 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
Bin Meng July 15, 2020, 1:05 a.m. UTC | #4
Hi Simon,

On Tue, Jul 14, 2020 at 9:25 PM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Bin,
>
> On Mon, 13 Jul 2020 at 23:01, Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Jul 14, 2020 at 9:28 AM Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Wed, Jul 8, 2020 at 11:33 AM Simon Glass <sjg at 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 <sjg at 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/?

No, there are still 11 patches in this series [34-44] that need rebase.

>
> I also have the final ACPI series in progress. I'd love to get it all
> done this cycle if possible.

Yep, I plan to send the PR soon with the complete part B & C series.

Regards,
Bin
diff mbox series

Patch

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);
+	acpi_dp_add_integer(dp, "sdmode-delay",
+			    dev_read_u32_default(dev, "sdmode-delay", 0));
+	acpi_dp_write(ctx, dp);
+
+	acpigen_pop_len(ctx); /* Device */
+	acpigen_pop_len(ctx); /* Scope */
+
+	return 0;
+}
+
+/* For now only X86 boards support NHLT */
+#ifdef CONFIG_X86
+static const struct nhlt_format_config max98357a_formats[] = {
+	/* 48 KHz 24-bits per sample. */
+	{
+		.num_channels = 2,
+		.sample_freq_khz = 48,
+		.container_bits_per_sample = 32,
+		.valid_bits_per_sample = 24,
+		.settings_file = "max98357-render-2ch-48khz-24b.dat",
+	},
+};
+
+static const struct nhlt_endp_descriptor max98357a_descriptors[] = {
+	{
+		.link = NHLT_LINK_SSP,
+		.device = NHLT_SSP_DEV_I2S,
+		.direction = NHLT_DIR_RENDER,
+		.vid = NHLT_VID,
+		.did = NHLT_DID_SSP,
+		.formats = max98357a_formats,
+		.num_formats = ARRAY_SIZE(max98357a_formats),
+	},
+};
+
+static int max98357a_acpi_setup_nhlt(const struct udevice *dev,
+				     struct acpi_ctx *ctx)
+{
+	u32 hwlink;
+	int ret;
+
+	if (dev_read_u32(dev, "acpi,audio-link", &hwlink))
+		return log_msg_ret("link", -EINVAL);
+
+	/* Virtual bus id of SSP links are the hardware port ids proper. */
+	ret = nhlt_add_ssp_endpoints(ctx->nhlt, hwlink, max98357a_descriptors,
+				     ARRAY_SIZE(max98357a_descriptors));
+	if (ret)
+		return log_msg_ret("add", ret);
+
+	return 0;
+}
+#endif
+
+struct acpi_ops max98357a_acpi_ops = {
+	.fill_ssdt	= max98357a_acpi_fill_ssdt,
+#ifdef CONFIG_X86
+	.setup_nhlt	= max98357a_acpi_setup_nhlt,
+#endif
+};
+
+static const struct audio_codec_ops max98357a_ops = {
+};
+
+static const struct udevice_id max98357a_ids[] = {
+	{ .compatible = "maxim,max98357a" },
+	{ }
+};
+
+U_BOOT_DRIVER(max98357a) = {
+	.name		= "max98357a",
+	.id		= UCLASS_AUDIO_CODEC,
+	.of_match	= max98357a_ids,
+	.ofdata_to_platdata	= max98357a_ofdata_to_platdata,
+	.ops		= &max98357a_ops,
+	ACPI_OPS_PTR(&max98357a_acpi_ops)
+};