[PATCH v2 17/44] sound: Add an ACPI driver for Maxim MAX98357ac
Simon Glass
sjg at chromium.org
Tue Jul 14 15:25:13 CEST 2020
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
More information about the U-Boot
mailing list