[PATCH v2 0/3] Add ASoC machine driver for Lenovo YB1 tablets

[PATCH v2 0/3] Add ASoC machine driver for Lenovo YB1 tablets

[Yauhen Kharuzhy]

This is the second iteration of my submission for sound support for Lenovo
    Yoga Book YB1-X90/X91 tablets.

    These are Cherry Trail-based platforms that use the RT5677 codec and
    TS3A227E jack detection IC.

    The YB1-X90 is an Android-designated device and has incorrect ACPI entries
    for many onboard peripherals, including sound devices. The YB1-X91 has a
    DSDT node for the codec, and the same node is shared with the TS3A227E
    jack detection IC (see below).

    Matching with ACPI entries is somewhat tricky; see the corresponding
    commit. Jack detection IC info is defined in the x86-android-tablets driver
    for both platforms to simplify the code.

    The machine driver is called 'cht_yogabook' because it has some
    hardcoded device-specific elements and it seems this is the only known
    Cherry Trail platform using the RT5677 codec. This naming is open
    to discussion: a more generic name like 'cht_rt5677' or 'cht_bsw_rt5677'
    may be better (requiring reworking of the driver to make it
    more generic with device-specific quirks).

v2:
 cht_yogabook driver:
 - Remove likely obsolete code from the driver: GPIO magic manipulations
   when enabling the speaker amplifier and msleep() calls when turning
   the speaker/headphones on and off.
 - Remove MICBIAS1 manipulation on headphone jack insertion (click
   prevention): the TS3A227E should handle it by itself.
 - Rename struct cht_mc_private to cht_yb_private.

 ACPI matching code:
 - Split the changes into two commits: reworking existing entries and adding
   new ones.
 - Use dmi_system_id.driver_data to specify matched machine data instead
   of callbacks.

For reference, there is an ACPI node for sound configuration from YB1-X91
DSDT below:

    HID: 10EC5677
_CRS resources:

  I2C devices:
  0: rt5677 codec
  1: ts3a227e jack detection IC

  GPIOs:
  0: rt5677 codec reset
  1: rt5677 codec pow-ldo2
  2: speaker enable

  INTs:
  0: rt5677 codec
  1: ts3a227e jack detection IC

  SPI device:
  0: rt5677 codec SPI connection

Decompiled fragment of ACPI dump:

    Device (RTEK)
    {
        Name (_ADR, Zero)  // _ADR: Address
        Name (_HID, "10EC5677")  // _HID: Hardware ID
        Name (_CID, "10EC5677")  // _CID: Compatible ID
        Name (_DDN, "Realtek IIS Audio Codec")  // _DDN: DOS Device Name
        Name (_SUB, "17AA7005")  // _SUB: Subsystem ID
        Name (_UID, One)  // _UID: Unique ID
        Name (_PR0, Package (0x01)  // _PR0: Power Resources for D0
        {
            CLK3
        })
        Name (CHAN, Package (0x02)
        {
            One,
            0x0124F800
        })
        Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
        {
            Name (SBUF, ResourceTemplate ()
            {
                I2cSerialBusV2 (0x002C, ControllerInitiated, 0x000186A0,
                    AddressingMode7Bit, "\\_SB.PCI0.I2C1",
                    0x00, ResourceConsumer, , Exclusive,
                    )
                I2cSerialBusV2 (0x003B, ControllerInitiated, 0x000186A0,
                    AddressingMode7Bit, "\\_SB.PCI0.I2C1",
                    0x00, ResourceConsumer, , Exclusive,
                    )
                GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
                    "\\_SB.GPO3", 0x00, ResourceConsumer, ,
                    )
                    {   // Pin list
                        0x0019
                    }
                GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
                    "\\_SB.GPO3", 0x00, ResourceConsumer, ,
                    )
                    {   // Pin list
                        0x0012
                    }
                GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
                    "\\_SB.GPO3", 0x00, ResourceConsumer, ,
                    )
                    {   // Pin list
                        0x0030
                    }
                GpioInt (Edge, ActiveLow, Exclusive, PullNone, 0x0000,
                    "\\_SB.GPO0", 0x00, ResourceConsumer, ,
                    )
                    {   // Pin list
                        0x005B
                    }
                GpioInt (Edge, ActiveLow, Exclusive, PullNone, 0x0000,
                    "\\_SB.GPO0", 0x00, ResourceConsumer, ,
                    )
                    {   // Pin list
                        0x004D
                    }
                SpiSerialBusV2 (0x0001, PolarityLow, FourWireMode, 0x08,
                    ControllerInitiated, 0x003D0900, ClockPolarityHigh,
                    ClockPhaseSecond, "\\_SB.PCI0.SPI1",
                    0x00, ResourceConsumer, , Exclusive,
                    )
            })
            Return (SBUF) /* \_SB_.PCI0.I2C1.RTEK._CRS.SBUF */
        }
    }

Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
---
Yauhen Kharuzhy (3):
      ASoC: Intel: soc-acpi-cht: Unify device quirks
      ASoC: Intel: soc-acpi-cht: Add Lenovo Yoga Book entries
      ASoC: Intel: cht_yogabook: Add driver for Lenovo Yoga Book tablets

 sound/soc/intel/boards/Kconfig                    |  15 +
 sound/soc/intel/boards/Makefile                   |   2 +
 sound/soc/intel/boards/cht_yogabook.c             | 551 ++++++++++++++++++++++
 sound/soc/intel/common/soc-acpi-intel-cht-match.c | 138 +++---
 4 files changed, 649 insertions(+), 57 deletions(-)
---
base-commit: 7f30250e792515b42130564cb52a9d218bad4cb3
change-id: 20260219-asoc-yogabook-v2-70c146371d20

Best regards,
-- 
Yauhen Kharuzhy

[PATCH v2 1/3] ASoC: Intel: soc-acpi-cht: Unify device quirks

[Yauhen Kharuzhy]

This file contains two types of quirks, both checking DMI for
machine-specific strings and returning machine data for a matching entry.

The first one, `cht_quirk`, is used to override the default entry for an
existing ACPI codec node if the node's info is invalid. It returns either
the matched machine data or the default entry if no match is found.

The second one, `cht_yt3_quirk_cb`, is used for devices (originally the
Lenovo Yoga Tab 3 Pro) without a valid codec DSDT entry. It is bound to
the SST ACPI node and returns either the matched machine data or NULL if
no match is found.

To allow adding new machine entries to the second case and to use a single
DMI match entry for both cases (for example, if two variants of one device
exist: one with a valid ACPI entry and one without, like the Lenovo Yoga
Book YB1-X91 and YB1-X90 - Windows and Android versions), reorganize
these quirks functions to use the same approach: machine data is set in
the matched dmi_system_id entry as driver_data field.

Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
---
 sound/soc/intel/common/soc-acpi-intel-cht-match.c | 100 +++++++++-------------
 1 file changed, 42 insertions(+), 58 deletions(-)

diff --git a/sound/soc/intel/common/soc-acpi-intel-cht-match.c b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
index e4c3492a0c28..57097c1d011e 100644
--- a/sound/soc/intel/common/soc-acpi-intel-cht-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
@@ -9,47 +9,63 @@
 #include <sound/soc-acpi.h>
 #include <sound/soc-acpi-intel-match.h>
 
-static unsigned long cht_machine_id;
-
-#define CHT_SURFACE_MACH 1
+static struct snd_soc_acpi_mach cht_surface_mach = {
+	.id = "10EC5640",
+	.drv_name = "cht-bsw-rt5645",
+	.fw_filename = "intel/fw_sst_22a8.bin",
+	.board = "cht-bsw",
+	.sof_tplg_filename = "sof-cht-rt5645.tplg",
+};
 
-static int cht_surface_quirk_cb(const struct dmi_system_id *id)
-{
-	cht_machine_id = CHT_SURFACE_MACH;
-	return 1;
-}
+static struct snd_soc_acpi_mach cht_lenovo_yoga_tab3_x90_mach = {
+	.id = "10WM5102",
+	.drv_name = "bytcr_wm5102",
+	.fw_filename = "intel/fw_sst_22a8.bin",
+	.board = "bytcr_wm5102",
+	.sof_tplg_filename = "sof-cht-wm5102.tplg",
+};
 
 static const struct dmi_system_id cht_table[] = {
 	{
-		.callback = cht_surface_quirk_cb,
+		.driver_data = (void *)&cht_surface_mach,
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
 		},
 	},
+	{
+		/*
+		 * The Lenovo Yoga Tab 3 Pro YT3-X90, with Android factory OS
+		 * has a buggy DSDT with the codec not being listed at all.
+		 */
+		.driver_data = (void *)&cht_lenovo_yoga_tab3_x90_mach,
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Intel Corporation"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "Blade3-10A-001"),
+		},
+	},
 	{ }
 };
 
-static struct snd_soc_acpi_mach cht_surface_mach = {
-	.id = "10EC5640",
-	.drv_name = "cht-bsw-rt5645",
-	.fw_filename = "intel/fw_sst_22a8.bin",
-	.board = "cht-bsw",
-	.sof_tplg_filename = "sof-cht-rt5645.tplg",
-};
-
 static struct snd_soc_acpi_mach *cht_quirk(void *arg)
 {
 	struct snd_soc_acpi_mach *mach = arg;
+	const struct dmi_system_id *match;
 
-	dmi_check_system(cht_table);
-
-	if (cht_machine_id == CHT_SURFACE_MACH)
-		return &cht_surface_mach;
+	match = dmi_first_match(cht_table);
+	if (match)
+		return (struct snd_soc_acpi_mach *)match->driver_data;
 	else
 		return mach;
 }
 
+static struct snd_soc_acpi_mach *cht_quirk_nocodec(void *arg)
+{
+	struct snd_soc_acpi_mach *mach = cht_quirk(arg);
+
+	return mach == arg ? NULL : mach;
+}
+
 /*
  * Some tablets with Android factory OS have buggy DSDTs with an ESSX8316 device
  * in the ACPI tables. While they are not using an ESS8316 codec. These DSDTs
@@ -75,38 +91,6 @@ static struct snd_soc_acpi_mach *cht_ess8316_quirk(void *arg)
 	return arg;
 }
 
-/*
- * The Lenovo Yoga Tab 3 Pro YT3-X90, with Android factory OS has a buggy DSDT
- * with the coded not being listed at all.
- */
-static const struct dmi_system_id lenovo_yoga_tab3_x90[] = {
-	{
-		/* Lenovo Yoga Tab 3 Pro YT3-X90, codec missing from DSDT */
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Intel Corporation"),
-			DMI_MATCH(DMI_PRODUCT_VERSION, "Blade3-10A-001"),
-		},
-	},
-	{ }
-};
-
-static struct snd_soc_acpi_mach cht_lenovo_yoga_tab3_x90_mach = {
-	.id = "10WM5102",
-	.drv_name = "bytcr_wm5102",
-	.fw_filename = "intel/fw_sst_22a8.bin",
-	.board = "bytcr_wm5102",
-	.sof_tplg_filename = "sof-cht-wm5102.tplg",
-};
-
-static struct snd_soc_acpi_mach *lenovo_yt3_x90_quirk(void *arg)
-{
-	if (dmi_check_system(lenovo_yoga_tab3_x90))
-		return &cht_lenovo_yoga_tab3_x90_mach;
-
-	/* Skip wildcard match snd_soc_acpi_intel_cherrytrail_machines[] entry */
-	return NULL;
-}
-
 static const struct snd_soc_acpi_codecs rt5640_comp_ids = {
 	.num_codecs = 2,
 	.codecs = { "10EC5640", "10EC3276" },
@@ -208,14 +192,14 @@ struct snd_soc_acpi_mach  snd_soc_acpi_intel_cherrytrail_machines[] = {
 		.sof_tplg_filename = "sof-cht-src-50khz-pcm512x.tplg",
 	},
 	/*
-	 * Special case for the Lenovo Yoga Tab 3 Pro YT3-X90 where the DSDT
-	 * misses the codec. Match on the SST id instead, lenovo_yt3_x90_quirk()
-	 * will return a YT3 specific mach or NULL when called on other hw,
-	 * skipping this entry.
+	 * Special case for devices where the DSDT misses the codec. Match on
+	 * the SST id instead, cht_quirk_nocodec() will return a
+	 * device-specific mach for matched device or NULL when called on other
+	 * hw, skipping this entry.
 	 */
 	{
 		.id = "808622A8",
-		.machine_quirk = lenovo_yt3_x90_quirk,
+		.machine_quirk = cht_quirk_nocodec,
 	},
 
 #if IS_ENABLED(CONFIG_SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH)

-- 
2.51.0

[PATCH v2 2/3] ASoC: Intel: soc-acpi-cht: Add Lenovo Yoga Book entries

[Yauhen Kharuzhy]

Lenovo Yoga Book YB1-X91 device uses a Cherry Trail SoC and has a valid
ACPI DSDT entry for the RT5677 codec. This entry has some non-standard
resource definitions, such as jack detection chip information, and
hardware has some additional GPIO controls so use 'cht-yogabook'
for the driver name instead of some default (like 'cht-bsw-rt5677').

Lenovo Yoga Book YB1-X90 device (Android version of the tablet) has the
same hardware configuration but lacks a valid ACPI DSDT entry for the
codec, so add DMI match data for it and use the same machine data as for
YB1-X91.

Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
---
 sound/soc/intel/common/soc-acpi-intel-cht-match.c | 40 +++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/sound/soc/intel/common/soc-acpi-intel-cht-match.c b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
index 57097c1d011e..8673ade66e9d 100644
--- a/sound/soc/intel/common/soc-acpi-intel-cht-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
@@ -17,6 +17,14 @@ static struct snd_soc_acpi_mach cht_surface_mach = {
 	.sof_tplg_filename = "sof-cht-rt5645.tplg",
 };
 
+static struct snd_soc_acpi_mach cht_yogabook_mach = {
+	.id = "10EC5677",
+	.drv_name = "cht-yogabook",
+	.fw_filename = "intel/fw_sst_22a8.bin",
+	.board = "cht-yogabook",
+	.sof_tplg_filename = "sof-cht-rt5677.tplg",
+};
+
 static struct snd_soc_acpi_mach cht_lenovo_yoga_tab3_x90_mach = {
 	.id = "10WM5102",
 	.drv_name = "bytcr_wm5102",
@@ -33,6 +41,24 @@ static const struct dmi_system_id cht_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
 		},
 	},
+	{
+		.ident = "Lenovo Yoga Book YB1-X91",
+		.driver_data = (void *)&cht_yogabook_mach,
+		/* YB1-X91L/F */
+		.matches = {
+			DMI_MATCH(DMI_PRODUCT_NAME, "Lenovo YB1-X91"),
+		}
+	},
+	{
+		.ident = "Lenovo Yoga Book YB1-X90",
+		.driver_data = (void *)&cht_yogabook_mach,
+		/* YB1-X90L/F, codec is not listed in DSDT */
+		.matches = {
+			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Intel Corporation"),
+			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "CHERRYVIEW D1 PLATFORM"),
+			DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "YETI-11"),
+		}
+	},
 	{
 		/*
 		 * The Lenovo Yoga Tab 3 Pro YT3-X90, with Android factory OS
@@ -121,6 +147,20 @@ struct snd_soc_acpi_mach  snd_soc_acpi_intel_cherrytrail_machines[] = {
 		.board = "cht-bsw",
 		.sof_tplg_filename = "sof-cht-rt5670.tplg",
 	},
+	/*
+	 * The only known Cherry Trail device with RT5677 codec and 10EC677
+	 * DSTD entry is the Lenovo Yoga Book YB1-X91. It has a device-specific
+	 * driver, so check DMI and use a machine quirk to override the default
+	 * (non-existent) machine driver.
+	 */
+	{
+		.id = "10EC5677",
+		.drv_name = "cht-bsw-rt5677",
+		.fw_filename = "intel/fw_sst_22a8.bin",
+		.board = "cht-bsw",
+		.machine_quirk = cht_quirk,
+		.sof_tplg_filename = "sof-cht-rt5677.tplg",
+	},
 	{
 		.comp_ids = &rt5645_comp_ids,
 		.drv_name = "cht-bsw-rt5645",

-- 
2.51.0

[PATCH v2 3/3] ASoC: Intel: cht_yogabook: Add driver for Lenovo Yoga Book tablets

[Yauhen Kharuzhy]

Add a new ASoC machine driver for Lenovo Yoga Book Intel Cherry
Trail-based tablets (YB1-X90F/L, YB1-X91F/L models).

This platform uses a Realtek ALC5677 codec accompanied by a TS3A227E jack
configuration detection IC.

The driver is based on the cht_bsw_rt5672.c mainline driver and the driver
from the vendor's Android kernel [1].

There are no other known Cherry Trail platforms using an RT5677 codec, so
the driver is named 'cht_yogabook', and some device-specific tricks are
hardcoded, such as jack events and additional GPIOs controlling the
speaker amplifier.

[1] https://github.com/deadman96385/android_kernel_lenovo_yeti/blob/master/sound/soc/intel/board/cht_bl_dpcm_rt5677.c

Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
---
 sound/soc/intel/boards/Kconfig        |  15 +
 sound/soc/intel/boards/Makefile       |   2 +
 sound/soc/intel/boards/cht_yogabook.c | 551 ++++++++++++++++++++++++++++++++++
 3 files changed, 568 insertions(+)

diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index c5942b5655d3..f7346ee085e7 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -237,6 +237,21 @@ config SND_SOC_INTEL_BYT_CHT_ES8316_MACH
 	  Say Y or m if you have such a device. This is a recommended option.
 	  If unsure select "N".
 
+config SND_SOC_INTEL_CHT_YOGABOOK_MACH
+	tristate "Cherrytrail-based Lenovo Yoga Book tablet"
+	depends on I2C && ACPI
+	depends on X86_INTEL_LPSS || COMPILE_TEST
+	depends on GPIOLIB || COMPILE_TEST
+	select SND_SOC_ACPI
+	select SND_SOC_RT5677
+	select SND_SOC_TS3A227E
+        help
+	  This adds support for ASoC machine driver for the Lenovo Yoga Book
+	  tablets based on CherryTrail platform with RT5677 audio codec, models
+	  YB1-X90F, YB1-X90L, YB1-X91F, YB1-X91L.
+          Say Y or m if you have such a device. This is a recommended option.
+          If unsure select "N".
+
 endif ## SND_SST_ATOM_HIFI2_PLATFORM || SND_SOC_SOF_BAYTRAIL
 
 if SND_SST_ATOM_HIFI2_PLATFORM
diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
index 25a1a9066cbf..58bd6029545b 100644
--- a/sound/soc/intel/boards/Makefile
+++ b/sound/soc/intel/boards/Makefile
@@ -15,6 +15,7 @@ snd-soc-sst-cht-bsw-nau8824-y := cht_bsw_nau8824.o
 snd-soc-sst-byt-cht-cx2072x-y := bytcht_cx2072x.o
 snd-soc-sst-byt-cht-da7213-y := bytcht_da7213.o
 snd-soc-sst-byt-cht-es8316-y := bytcht_es8316.o
+snd-soc-sst-cht-yogabook-objs := cht_yogabook.o
 snd-soc-sst-byt-cht-nocodec-y := bytcht_nocodec.o
 snd-soc-sof_rt5682-y := sof_rt5682.o
 snd-soc-sof_cs42l42-y := sof_cs42l42.o
@@ -47,6 +48,7 @@ obj-$(CONFIG_SND_SOC_INTEL_CHT_BSW_NAU8824_MACH) += snd-soc-sst-cht-bsw-nau8824.
 obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_CX2072X_MACH) += snd-soc-sst-byt-cht-cx2072x.o
 obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_DA7213_MACH) += snd-soc-sst-byt-cht-da7213.o
 obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_ES8316_MACH) += snd-soc-sst-byt-cht-es8316.o
+obj-$(CONFIG_SND_SOC_INTEL_CHT_YOGABOOK_MACH) += snd-soc-sst-cht-yogabook.o
 obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH) += snd-soc-sst-byt-cht-nocodec.o
 obj-$(CONFIG_SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH) += snd-soc-skl_hda_dsp.o
 obj-$(CONFIG_SND_SOC_INTEL_EHL_RT5660_MACH) += snd-soc-ehl-rt5660.o
diff --git a/sound/soc/intel/boards/cht_yogabook.c b/sound/soc/intel/boards/cht_yogabook.c
new file mode 100644
index 000000000000..9d945cad5660
--- /dev/null
+++ b/sound/soc/intel/boards/cht_yogabook.c
@@ -0,0 +1,551 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  cht_yogabook.c - ASoc Machine driver for Lenovo Yoga Book YB1-X90/X91
+ *  tablets, based on Intel Cherrytrail platform with RT5677 codec.
+ *
+ *  Copyright (C) 2026 Yauhen Kharuzhy <jekhor@gmail.com>
+ *
+ *  Based on cht_bsw_rt5672.c:
+ *  Copyright (C) 2014 Intel Corp
+ *  Author: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
+ *          Mengdong Lin <mengdong.lin@intel.com>
+ *
+ *  And based on the cht_bl_dpcm_rt5677.c from the Lenovo's Android kernel:
+ *  Copyright (C) 2014 Intel Corp
+ *  Author: Mythri P K <mythri.p.k@intel.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/dmi.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <sound/jack.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc-acpi.h>
+#include <sound/soc.h>
+#include "../../codecs/rt5677.h"
+#include "../../codecs/ts3a227e.h"
+#include "../atom/sst-atom-controls.h"
+
+#define RT5677_I2C_DEFAULT	"i2c-rt5677"
+
+/* The platform clock #3 outputs 19.2Mhz clock to codec as I2S MCLK */
+#define CHT_PLAT_CLK_3_HZ	19200000
+#define CHT_CODEC_DAI		"rt5677-aif1"
+
+struct cht_yb_private {
+	char codec_name[SND_ACPI_I2C_ID_LEN];
+	struct snd_soc_jack jack;
+	struct clk *mclk;
+	struct gpio_desc *gpio_spk_en1;
+	struct gpio_desc *gpio_spk_en2;
+	struct gpio_desc *gpio_hp_en;
+};
+
+static int cht_yb_platform_clock_control(struct snd_soc_dapm_widget *w,
+					 struct snd_kcontrol *k, int  event)
+{
+	struct snd_soc_card *card = snd_soc_dapm_to_card(w->dapm);
+	struct snd_soc_dai *codec_dai;
+	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	codec_dai = snd_soc_card_get_codec_dai(card, CHT_CODEC_DAI);
+	if (!codec_dai) {
+		dev_err(card->dev,
+			"Codec dai not found; Unable to set platform clock\n");
+		return -EIO;
+	}
+
+	if (SND_SOC_DAPM_EVENT_ON(event)) {
+		if (ctx->mclk) {
+			ret = clk_prepare_enable(ctx->mclk);
+			if (ret < 0) {
+				dev_err(card->dev,
+					"could not configure MCLK state");
+				return ret;
+			}
+		}
+
+		/* set codec PLL source to the 19.2MHz platform clock (MCLK) */
+		ret = snd_soc_dai_set_pll(codec_dai, 0, RT5677_PLL1_S_MCLK,
+					  CHT_PLAT_CLK_3_HZ, 48000 * 512);
+		if (ret < 0) {
+			dev_err(card->dev, "can't set codec pll: %d\n", ret);
+			return ret;
+		}
+
+		/* set codec sysclk source to PLL */
+		ret = snd_soc_dai_set_sysclk(codec_dai, RT5677_SCLK_S_PLL1,
+					     48000 * 512, SND_SOC_CLOCK_IN);
+		if (ret < 0) {
+			dev_err(card->dev, "can't set codec sysclk: %d\n", ret);
+			return ret;
+		}
+	} else {
+		/* Set codec sysclk source to its internal clock because codec
+		 * PLL will be off when idle and MCLK will also be off by ACPI
+		 * when codec is runtime suspended. Codec needs clock for jack
+		 * detection and button press.
+		 */
+		snd_soc_dai_set_sysclk(codec_dai, RT5677_SCLK_S_RCCLK,
+				       48000 * 512, SND_SOC_CLOCK_IN);
+
+		if (ctx->mclk)
+			clk_disable_unprepare(ctx->mclk);
+	}
+	return 0;
+}
+
+static int cht_yb_hp_event(struct snd_soc_dapm_widget *w,
+			   struct snd_kcontrol *k, int event)
+{
+	struct snd_soc_card *card = snd_soc_dapm_to_card(w->dapm);
+	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(card);
+
+	dev_dbg(card->dev, "HP event: %s\n",
+		SND_SOC_DAPM_EVENT_ON(event) ? "ON" : "OFF");
+
+	gpiod_set_value_cansleep(ctx->gpio_hp_en, SND_SOC_DAPM_EVENT_ON(event));
+
+	return 0;
+}
+
+static int cht_yb_spk_event(struct snd_soc_dapm_widget *w,
+			    struct snd_kcontrol *k, int event)
+{
+	struct snd_soc_card *card = snd_soc_dapm_to_card(w->dapm);
+	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(card);
+
+	dev_dbg(card->dev, "SPK event: %s\n",
+		SND_SOC_DAPM_EVENT_ON(event) ? "ON" : "OFF");
+
+	gpiod_set_value_cansleep(ctx->gpio_spk_en1,
+				 SND_SOC_DAPM_EVENT_ON(event));
+	gpiod_set_value_cansleep(ctx->gpio_spk_en2,
+				 SND_SOC_DAPM_EVENT_ON(event));
+
+	return 0;
+}
+
+static const struct snd_soc_dapm_widget cht_yb_dapm_widgets[] = {
+	SND_SOC_DAPM_HP("Headphone", cht_yb_hp_event),
+	SND_SOC_DAPM_MIC("Headset Mic", NULL),
+	SND_SOC_DAPM_MIC("Int Mic", NULL),
+	SND_SOC_DAPM_SPK("Speaker", cht_yb_spk_event),
+	SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
+			    cht_yb_platform_clock_control,
+			    SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+};
+
+static const struct snd_soc_dapm_route cht_yb_audio_map[] = {
+	{"IN1P", NULL, "Headset Mic"},
+	{"IN1N", NULL, "Headset Mic"},
+	{"DMIC L1", NULL, "Int Mic"},
+	{"DMIC R1", NULL, "Int Mic"},
+	{"Headphone", NULL, "LOUT1"},
+	{"Headphone", NULL, "LOUT2"},
+	{"Speaker", NULL, "LOUT1"},
+	{"Speaker", NULL, "LOUT2"},
+
+	{"AIF1 Playback", NULL, "ssp2 Tx"},
+	{"ssp2 Tx", NULL, "codec_out0"},
+	{"ssp2 Tx", NULL, "codec_out1"},
+	{"codec_in0", NULL, "ssp2 Rx"},
+	{"codec_in1", NULL, "ssp2 Rx"},
+	{"ssp2 Rx", NULL, "AIF1 Capture"},
+	{"Headphone", NULL, "Platform Clock"},
+	{"Speaker", NULL, "Platform Clock"},
+	{"Headset Mic", NULL, "Platform Clock"},
+	{"Int Mic", NULL, "Platform Clock"},
+};
+
+static const struct snd_kcontrol_new cht_mc_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Headphone"),
+	SOC_DAPM_PIN_SWITCH("Headset Mic"),
+	SOC_DAPM_PIN_SWITCH("Int Mic"),
+	SOC_DAPM_PIN_SWITCH("Speaker"),
+};
+
+static int cht_yb_aif1_hw_params(struct snd_pcm_substream *substream,
+				 struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
+	int ret;
+
+	/* set codec PLL source to the 19.2MHz platform clock (MCLK) */
+	ret = snd_soc_dai_set_pll(codec_dai, 0, RT5677_PLL1_S_MCLK,
+				  CHT_PLAT_CLK_3_HZ, params_rate(params) * 512);
+	if (ret < 0) {
+		dev_err(rtd->dev, "can't set codec pll: %d\n", ret);
+		return ret;
+	}
+
+	/* set codec sysclk source to PLL */
+	ret = snd_soc_dai_set_sysclk(codec_dai, RT5677_SCLK_S_PLL1,
+				     params_rate(params) * 512,
+				     SND_SOC_CLOCK_IN);
+	if (ret < 0) {
+		dev_err(rtd->dev, "can't set codec sysclk: %d\n", ret);
+		return ret;
+	}
+	/*
+	 * Default mode for SSP configuration is TDM 4 slot
+	 */
+	ret = snd_soc_dai_set_fmt(codec_dai,
+				  SND_SOC_DAIFMT_DSP_B |
+				  SND_SOC_DAIFMT_IB_NF |
+				  SND_SOC_DAIFMT_CBC_CFC);
+	if (ret < 0) {
+		dev_err(codec_dai->dev, "can't set format to TDM %d\n", ret);
+		return ret;
+	}
+
+	/* TDM 4 slots 24 bit, set Rx & Tx bitmask to 4 active slots */
+	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xF, 0xF, 4, 25);
+	if (ret < 0) {
+		dev_err(rtd->dev, "can't set codec TDM slot %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int cht_yb_codec_init(struct snd_soc_pcm_runtime *runtime)
+{
+	int ret = 0;
+	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(runtime, 0);
+	struct snd_soc_component *component = codec_dai->component;
+	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(runtime->card);
+
+	/* Enable codec ASRC function for Stereo DAC/Stereo1 ADC/DMIC/I2S1.
+	 * The ASRC clock source is clk_i2s1_asrc.
+	 */
+	rt5677_sel_asrc_clk_src(component, RT5677_DA_STEREO_FILTER |
+			RT5677_AD_STEREO1_FILTER | RT5677_I2S1_SOURCE,
+			RT5677_CLK_SEL_I2S1_ASRC);
+	/* Enable codec ASRC function for Mono ADC L.
+	 * The ASRC clock source is clk_sys2_asrc.
+	 */
+	rt5677_sel_asrc_clk_src(component, RT5677_AD_MONO_L_FILTER,
+				RT5677_CLK_SEL_SYS2);
+
+	ctx->gpio_spk_en1 = devm_gpiod_get(component->dev, "speaker-enable",
+					   GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->gpio_spk_en1)) {
+		dev_err(component->dev, "Can't find speaker enable GPIO\n");
+		return PTR_ERR(ctx->gpio_spk_en1);
+	}
+
+	ctx->gpio_spk_en2 = devm_gpiod_get(component->dev, "speaker-enable2",
+					   GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->gpio_spk_en2)) {
+		dev_err(component->dev, "Can't find speaker enable 2 GPIO\n");
+		return PTR_ERR(ctx->gpio_spk_en2);
+	}
+
+	ctx->gpio_hp_en = devm_gpiod_get(component->dev, "headphone-enable",
+					 GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->gpio_hp_en)) {
+		dev_err(component->dev, "Can't find headphone enable GPIO\n");
+		return PTR_ERR(ctx->gpio_hp_en);
+	}
+
+	if (ctx->mclk) {
+		/*
+		 * The firmware might enable the clock at
+		 * boot (this information may or may not
+		 * be reflected in the enable clock register).
+		 * To change the rate we must disable the clock
+		 * first to cover these cases. Due to common
+		 * clock framework restrictions that do not allow
+		 * to disable a clock that has not been enabled,
+		 * we need to enable the clock first.
+		 */
+		ret = clk_prepare_enable(ctx->mclk);
+		if (!ret)
+			clk_disable_unprepare(ctx->mclk);
+
+		ret = clk_set_rate(ctx->mclk, CHT_PLAT_CLK_3_HZ);
+
+		if (ret) {
+			dev_err(runtime->dev, "unable to set MCLK rate\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int cht_yb_codec_fixup(struct snd_soc_pcm_runtime *rtd,
+			      struct snd_pcm_hw_params *params)
+{
+	struct snd_interval *rate = hw_param_interval(params,
+			SNDRV_PCM_HW_PARAM_RATE);
+	struct snd_interval *channels = hw_param_interval(params,
+						SNDRV_PCM_HW_PARAM_CHANNELS);
+
+	/* The DSP will convert the FE rate to 48k, stereo, 24bits */
+	rate->min = rate->max = 48000;
+	channels->min = channels->max = 2;
+
+	/* set SSP2 to 24-bit */
+	params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
+
+	return 0;
+}
+
+static struct snd_soc_jack_pin cht_yb_jack_pins[] = {
+	{
+		.pin = "Headphone",
+		.mask = SND_JACK_HEADPHONE,
+	},
+	{
+		.pin = "Headset Mic",
+		.mask = SND_JACK_MICROPHONE,
+	},
+};
+
+static int cht_yb_headset_init(struct snd_soc_component *component)
+{
+	struct snd_soc_card *card = component->card;
+	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(card);
+	struct snd_soc_jack *jack = &ctx->jack;
+	int jack_type;
+	int ret;
+
+	/*
+	 * TS3A227E supports 4 butons headset detection
+	 * KEY_MEDIA
+	 * KEY_VOICECOMMAND
+	 * KEY_VOLUMEUP
+	 * KEY_VOLUMEDOWN
+	 */
+	jack_type = SND_JACK_HEADPHONE | SND_JACK_MICROPHONE |
+		    SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+		    SND_JACK_BTN_2 | SND_JACK_BTN_3;
+
+	ret = snd_soc_card_jack_new_pins(card, "Headset Jack",
+					 jack_type, jack,
+					 cht_yb_jack_pins,
+					 ARRAY_SIZE(cht_yb_jack_pins));
+	if (ret) {
+		dev_err(card->dev, "Headset Jack creation failed %d\n", ret);
+		return ret;
+	}
+
+	return ts3a227e_enable_jack_detect(component, jack);
+}
+
+static int cht_yb_aif1_startup(struct snd_pcm_substream *substream)
+{
+	return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_RATE, 48000);
+}
+
+static const struct snd_soc_ops cht_yb_aif1_ops = {
+	.startup = cht_yb_aif1_startup,
+};
+
+static const struct snd_soc_ops cht_yb_be_ssp2_ops = {
+	.hw_params = cht_yb_aif1_hw_params,
+};
+
+static struct snd_soc_aux_dev cht_yb_headset_dev = {
+	.dlc = COMP_AUX("i2c-ts3a227e"),
+	.init = cht_yb_headset_init,
+};
+
+SND_SOC_DAILINK_DEF(dummy,
+	DAILINK_COMP_ARRAY(COMP_DUMMY()));
+
+SND_SOC_DAILINK_DEF(media,
+	DAILINK_COMP_ARRAY(COMP_CPU("media-cpu-dai")));
+
+SND_SOC_DAILINK_DEF(deepbuffer,
+	DAILINK_COMP_ARRAY(COMP_CPU("deepbuffer-cpu-dai")));
+
+SND_SOC_DAILINK_DEF(ssp2_port,
+	DAILINK_COMP_ARRAY(COMP_CPU("ssp2-port")));
+SND_SOC_DAILINK_DEF(ssp2_codec,
+	DAILINK_COMP_ARRAY(COMP_CODEC(RT5677_I2C_DEFAULT, "rt5677-aif1")));
+
+SND_SOC_DAILINK_DEF(platform,
+	DAILINK_COMP_ARRAY(COMP_PLATFORM("sst-mfld-platform")));
+
+static struct snd_soc_dai_link cht_yb_dailink[] = {
+	/* Front End DAI links */
+	[MERR_DPCM_AUDIO] = {
+		.name = "Audio Port",
+		.stream_name = "Audio",
+		.nonatomic = true,
+		.dynamic = 1,
+		.ops = &cht_yb_aif1_ops,
+		SND_SOC_DAILINK_REG(media, dummy, platform),
+	},
+	[MERR_DPCM_DEEP_BUFFER] = {
+		.name = "Deep-Buffer Audio Port",
+		.stream_name = "Deep-Buffer Audio",
+		.nonatomic = true,
+		.dynamic = 1,
+		.playback_only = 1,
+		.ops = &cht_yb_aif1_ops,
+		SND_SOC_DAILINK_REG(deepbuffer, dummy, platform),
+	},
+
+	/* Back End DAI links */
+	{
+		/* SSP2 - Codec */
+		.name = "SSP2-Codec",
+		.id = 0,
+		.no_pcm = 1,
+		.nonatomic = true,
+		.init = cht_yb_codec_init,
+		.be_hw_params_fixup = cht_yb_codec_fixup,
+		.ops = &cht_yb_be_ssp2_ops,
+		SND_SOC_DAILINK_REG(ssp2_port, ssp2_codec, platform),
+	},
+};
+
+/* SoC card */
+static struct snd_soc_card snd_soc_card_cht_yb = {
+	.owner = THIS_MODULE,
+	.dai_link = cht_yb_dailink,
+	.num_links = ARRAY_SIZE(cht_yb_dailink),
+	.aux_dev = &cht_yb_headset_dev,
+	.num_aux_devs = 1,
+	.dapm_widgets = cht_yb_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(cht_yb_dapm_widgets),
+	.dapm_routes = cht_yb_audio_map,
+	.num_dapm_routes = ARRAY_SIZE(cht_yb_audio_map),
+	.controls = cht_mc_controls,
+	.num_controls = ARRAY_SIZE(cht_mc_controls),
+};
+
+static const struct acpi_gpio_params speaker_enable_gpio = { 2, 0, false };
+static const struct acpi_gpio_mapping cht_yb_gpios[] = {
+	{ "speaker-enable-gpios", &speaker_enable_gpio, 1 },
+	{ NULL }
+};
+
+#define SOF_CARD_NAME "cht yogabook"
+#define SOF_DRIVER_NAME "SOF"
+
+#define CARD_NAME "cht-yogabook"
+#define DRIVER_NAME NULL
+
+static int snd_cht_yb_mc_probe(struct platform_device *pdev)
+{
+	int ret_val = 0;
+	struct cht_yb_private *drv;
+	struct snd_soc_acpi_mach *mach = pdev->dev.platform_data;
+	const char *platform_name;
+	struct acpi_device *adev;
+	struct device *codec_dev;
+	bool sof_parent;
+	int i;
+
+	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
+	if (!drv)
+		return -ENOMEM;
+
+	strscpy(drv->codec_name, RT5677_I2C_DEFAULT);
+
+	/* fixup codec name based on HID if ACPI node is present */
+	adev = acpi_dev_get_first_match_dev(mach->id, NULL, -1);
+	if (adev) {
+		snprintf(drv->codec_name, sizeof(drv->codec_name),
+			 "i2c-%s", acpi_dev_name(adev));
+		dev_info(&pdev->dev, "real codec name: %s\n", drv->codec_name);
+
+		put_device(&adev->dev);
+		for (i = 0; i < ARRAY_SIZE(cht_yb_dailink); i++) {
+			if (cht_yb_dailink[i].codecs->name &&
+			    !strcmp(cht_yb_dailink[i].codecs->name,
+				    RT5677_I2C_DEFAULT)) {
+				cht_yb_dailink[i].codecs->name = drv->codec_name;
+				break;
+			}
+		}
+	}
+
+	codec_dev = bus_find_device_by_name(&i2c_bus_type, NULL,
+					    drv->codec_name);
+	if (!codec_dev)
+		return -EPROBE_DEFER;
+
+	if (adev) {
+		ret_val = devm_acpi_dev_add_driver_gpios(codec_dev,
+							 cht_yb_gpios);
+		if (ret_val)
+			dev_warn(&pdev->dev,
+				 "Unable to add GPIO mapping table: %d\n",
+				 ret_val);
+	}
+
+	/* override platform name, if required */
+	snd_soc_card_cht_yb.dev = &pdev->dev;
+	platform_name = mach->mach_params.platform;
+
+	ret_val = snd_soc_fixup_dai_links_platform_name(&snd_soc_card_cht_yb,
+							platform_name);
+	if (ret_val) {
+		dev_err(&pdev->dev,
+			"snd_soc_fixup_dai_links_platform_name failed: %d\n",
+			ret_val);
+		return ret_val;
+	}
+
+	drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
+	if (IS_ERR(drv->mclk)) {
+		dev_err(&pdev->dev,
+			"Failed to get MCLK from pmc_plt_clk_3: %ld\n",
+			PTR_ERR(drv->mclk));
+		return PTR_ERR(drv->mclk);
+	}
+	snd_soc_card_set_drvdata(&snd_soc_card_cht_yb, drv);
+
+	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
+
+	/* set the card and driver name */
+	if (sof_parent) {
+		snd_soc_card_cht_yb.name = SOF_CARD_NAME;
+		snd_soc_card_cht_yb.driver_name = SOF_DRIVER_NAME;
+	} else {
+		snd_soc_card_cht_yb.name = CARD_NAME;
+		snd_soc_card_cht_yb.driver_name = DRIVER_NAME;
+	}
+
+	/* register the soc card */
+	snd_soc_card_cht_yb.dev = &pdev->dev;
+	ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht_yb);
+	if (ret_val) {
+		dev_err(&pdev->dev,
+			"snd_soc_register_card failed %d\n", ret_val);
+		return ret_val;
+	}
+	platform_set_drvdata(pdev, &snd_soc_card_cht_yb);
+
+	return ret_val;
+}
+
+static struct platform_driver snd_cht_yb_mc_driver = {
+	.driver = {
+		.name = "cht-yogabook",
+	},
+	.probe = snd_cht_yb_mc_probe,
+};
+
+module_platform_driver(snd_cht_yb_mc_driver);
+
+MODULE_DESCRIPTION("Lenovo Yoga Book YB1-X9x machine driver");
+MODULE_AUTHOR("Yauhen Kharuzhy");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:cht-yogabook");

-- 
2.51.0

Re: [PATCH v2 3/3] ASoC: Intel: cht_yogabook: Add driver for Lenovo Yoga Book tablets

[Cezary Rojewski]

On 2026-03-01 10:33 PM, Yauhen Kharuzhy wrote:
> Add a new ASoC machine driver for Lenovo Yoga Book Intel Cherry
> Trail-based tablets (YB1-X90F/L, YB1-X91F/L models).
> 
> This platform uses a Realtek ALC5677 codec accompanied by a TS3A227E jack
> configuration detection IC.
> 
> The driver is based on the cht_bsw_rt5672.c mainline driver and the driver
> from the vendor's Android kernel [1].
> 
> There are no other known Cherry Trail platforms using an RT5677 codec, so
> the driver is named 'cht_yogabook', and some device-specific tricks are
> hardcoded, such as jack events and additional GPIOs controlling the
> speaker amplifier.

I'd switch to more specific naming. From the Intel-maintainer point of 
view, it's easier to navigate between configurations when 
<platform>-<codec> naming is in use. TBH, we do not see "yogabook", we 
configuration composed of Cherrytrail-based device and rt5677 I2C codec.

For the entire driver:
- fix alignments
- fix char-per-line limit, submission rules expect 100c-per-line limit

Below additional comments.

> 
> [1] https://github.com/deadman96385/android_kernel_lenovo_yeti/blob/master/sound/soc/intel/board/cht_bl_dpcm_rt5677.c
> 
> Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>

...


> diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
> index 25a1a9066cbf..58bd6029545b 100644
> --- a/sound/soc/intel/boards/Makefile
> +++ b/sound/soc/intel/boards/Makefile
> @@ -15,6 +15,7 @@ snd-soc-sst-cht-bsw-nau8824-y := cht_bsw_nau8824.o
>   snd-soc-sst-byt-cht-cx2072x-y := bytcht_cx2072x.o
>   snd-soc-sst-byt-cht-da7213-y := bytcht_da7213.o
>   snd-soc-sst-byt-cht-es8316-y := bytcht_es8316.o
> +snd-soc-sst-cht-yogabook-objs := cht_yogabook.o

Let's follow recommendation from Takashi [1]. Suffix "-objs" is obsolete.

[1]: https://lore.kernel.org/all/20240507155540.24815-13-tiwai@suse.de/

>   snd-soc-sst-byt-cht-nocodec-y := bytcht_nocodec.o
>   snd-soc-sof_rt5682-y := sof_rt5682.o
>   snd-soc-sof_cs42l42-y := sof_cs42l42.o
> @@ -47,6 +48,7 @@ obj-$(CONFIG_SND_SOC_INTEL_CHT_BSW_NAU8824_MACH) += snd-soc-sst-cht-bsw-nau8824.
>   obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_CX2072X_MACH) += snd-soc-sst-byt-cht-cx2072x.o
>   obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_DA7213_MACH) += snd-soc-sst-byt-cht-da7213.o
>   obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_ES8316_MACH) += snd-soc-sst-byt-cht-es8316.o
> +obj-$(CONFIG_SND_SOC_INTEL_CHT_YOGABOOK_MACH) += snd-soc-sst-cht-yogabook.o
>   obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH) += snd-soc-sst-byt-cht-nocodec.o
>   obj-$(CONFIG_SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH) += snd-soc-skl_hda_dsp.o
>   obj-$(CONFIG_SND_SOC_INTEL_EHL_RT5660_MACH) += snd-soc-ehl-rt5660.o
> diff --git a/sound/soc/intel/boards/cht_yogabook.c b/sound/soc/intel/boards/cht_yogabook.c
> new file mode 100644
> index 000000000000..9d945cad5660
> --- /dev/null
> +++ b/sound/soc/intel/boards/cht_yogabook.c
> @@ -0,0 +1,551 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + *  cht_yogabook.c - ASoc Machine driver for Lenovo Yoga Book YB1-X90/X91
> + *  tablets, based on Intel Cherrytrail platform with RT5677 codec.
> + *
> + *  Copyright (C) 2026 Yauhen Kharuzhy <jekhor@gmail.com>
> + *
> + *  Based on cht_bsw_rt5672.c:
> + *  Copyright (C) 2014 Intel Corp
> + *  Author: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> + *          Mengdong Lin <mengdong.lin@intel.com>
> + *
> + *  And based on the cht_bl_dpcm_rt5677.c from the Lenovo's Android kernel:
> + *  Copyright (C) 2014 Intel Corp
> + *  Author: Mythri P K <mythri.p.k@intel.com>

Not sure about the boiler plate here. Mentioning people is OK but the 
emails are all obsolete. Such information does not help anyone. Perhaps 
"based on XYZ" is enough.

> + */
> +
> +#include <linux/clk.h>
> +#include <linux/dmi.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/machine.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <sound/jack.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc-acpi.h>
> +#include <sound/soc.h>
> +#include "../../codecs/rt5677.h"
> +#include "../../codecs/ts3a227e.h"
> +#include "../atom/sst-atom-controls.h"
> +
> +#define RT5677_I2C_DEFAULT	"i2c-rt5677"
> +
> +/* The platform clock #3 outputs 19.2Mhz clock to codec as I2S MCLK */
> +#define CHT_PLAT_CLK_3_HZ	19200000
> +#define CHT_CODEC_DAI		"rt5677-aif1"
> +
> +struct cht_yb_private {
> +	char codec_name[SND_ACPI_I2C_ID_LEN];
> +	struct snd_soc_jack jack;
> +	struct clk *mclk;
> +	struct gpio_desc *gpio_spk_en1;
> +	struct gpio_desc *gpio_spk_en2;
> +	struct gpio_desc *gpio_hp_en;
> +};
> +
> +static int cht_yb_platform_clock_control(struct snd_soc_dapm_widget *w,
> +					 struct snd_kcontrol *k, int  event)

Please replace 'k' with 'kctl' for the entire file.

> +{
> +	struct snd_soc_card *card = snd_soc_dapm_to_card(w->dapm);
> +	struct snd_soc_dai *codec_dai;
> +	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(card);
> +	int ret;
> +
> +	codec_dai = snd_soc_card_get_codec_dai(card, CHT_CODEC_DAI);
> +	if (!codec_dai) {
> +		dev_err(card->dev,
> +			"Codec dai not found; Unable to set platform clock\n");
> +		return -EIO;
> +	}
> +
> +	if (SND_SOC_DAPM_EVENT_ON(event)) {
> +		if (ctx->mclk) {
> +			ret = clk_prepare_enable(ctx->mclk);
> +			if (ret < 0) {
> +				dev_err(card->dev,
> +					"could not configure MCLK state");
> +				return ret;
> +			}
> +		}
> +
> +		/* set codec PLL source to the 19.2MHz platform clock (MCLK) */
> +		ret = snd_soc_dai_set_pll(codec_dai, 0, RT5677_PLL1_S_MCLK,
> +					  CHT_PLAT_CLK_3_HZ, 48000 * 512);
> +		if (ret < 0) {
> +			dev_err(card->dev, "can't set codec pll: %d\n", ret);
> +			return ret;
> +		}
> +
> +		/* set codec sysclk source to PLL */
> +		ret = snd_soc_dai_set_sysclk(codec_dai, RT5677_SCLK_S_PLL1,
> +					     48000 * 512, SND_SOC_CLOCK_IN);
> +		if (ret < 0) {
> +			dev_err(card->dev, "can't set codec sysclk: %d\n", ret);
> +			return ret;
> +		}
> +	} else {
> +		/* Set codec sysclk source to its internal clock because codec
> +		 * PLL will be off when idle and MCLK will also be off by ACPI
> +		 * when codec is runtime suspended. Codec needs clock for jack
> +		 * detection and button press.
> +		 */
> +		snd_soc_dai_set_sysclk(codec_dai, RT5677_SCLK_S_RCCLK,
> +				       48000 * 512, SND_SOC_CLOCK_IN);
> +
> +		if (ctx->mclk)
> +			clk_disable_unprepare(ctx->mclk);
> +	}

The entire function is made of if-statement. I'd split this into:

xyz_platform_clock_control()
|_ xyz_platform_clock_enable()
|_ xyz_platform_clock_disable()


> +	return 0;
> +}
> +
> +static int cht_yb_hp_event(struct snd_soc_dapm_widget *w,
> +			   struct snd_kcontrol *k, int event)
> +{
> +	struct snd_soc_card *card = snd_soc_dapm_to_card(w->dapm);
> +	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(card);
> +
> +	dev_dbg(card->dev, "HP event: %s\n",
> +		SND_SOC_DAPM_EVENT_ON(event) ? "ON" : "OFF");

These dev_dbg() can be dropped. Function tracer or DAPM events found 
within tracing/events are sufficient.

> +
> +	gpiod_set_value_cansleep(ctx->gpio_hp_en, SND_SOC_DAPM_EVENT_ON(event));
> +
> +	return 0;
> +}
> +
> +static int cht_yb_spk_event(struct snd_soc_dapm_widget *w,
> +			    struct snd_kcontrol *k, int event)
> +{
> +	struct snd_soc_card *card = snd_soc_dapm_to_card(w->dapm);
> +	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(card);
> +
> +	dev_dbg(card->dev, "SPK event: %s\n",
> +		SND_SOC_DAPM_EVENT_ON(event) ? "ON" : "OFF");

Ditto.

> +
> +	gpiod_set_value_cansleep(ctx->gpio_spk_en1,
> +				 SND_SOC_DAPM_EVENT_ON(event));
> +	gpiod_set_value_cansleep(ctx->gpio_spk_en2,
> +				 SND_SOC_DAPM_EVENT_ON(event));
> +
> +	return 0;
> +}

...

> +static int cht_yb_codec_init(struct snd_soc_pcm_runtime *runtime)
> +{
> +	int ret = 0;

Move as last.

> +	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(runtime, 0);
> +	struct snd_soc_component *component = codec_dai->component;
> +	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(runtime->card);
> +
> +	/* Enable codec ASRC function for Stereo DAC/Stereo1 ADC/DMIC/I2S1.
> +	 * The ASRC clock source is clk_i2s1_asrc.
> +	 */
> +	rt5677_sel_asrc_clk_src(component, RT5677_DA_STEREO_FILTER |
> +			RT5677_AD_STEREO1_FILTER | RT5677_I2S1_SOURCE,
> +			RT5677_CLK_SEL_I2S1_ASRC);
> +	/* Enable codec ASRC function for Mono ADC L.
> +	 * The ASRC clock source is clk_sys2_asrc.
> +	 */
> +	rt5677_sel_asrc_clk_src(component, RT5677_AD_MONO_L_FILTER,
> +				RT5677_CLK_SEL_SYS2);
> +
> +	ctx->gpio_spk_en1 = devm_gpiod_get(component->dev, "speaker-enable",
> +					   GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->gpio_spk_en1)) {
> +		dev_err(component->dev, "Can't find speaker enable GPIO\n");
> +		return PTR_ERR(ctx->gpio_spk_en1);
> +	}
> +
> +	ctx->gpio_spk_en2 = devm_gpiod_get(component->dev, "speaker-enable2",
> +					   GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->gpio_spk_en2)) {
> +		dev_err(component->dev, "Can't find speaker enable 2 GPIO\n");
> +		return PTR_ERR(ctx->gpio_spk_en2);
> +	}
> +
> +	ctx->gpio_hp_en = devm_gpiod_get(component->dev, "headphone-enable",
> +					 GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->gpio_hp_en)) {
> +		dev_err(component->dev, "Can't find headphone enable GPIO\n");

The messages here are misleading. "Can't find ..." when devm_gpiod_get() 
returns different IS_ERR() than -ENOENT is incorrect. I'd just state 
that XYZ operation failed and append the return code. Scales nicely into 
the future.

> +		return PTR_ERR(ctx->gpio_hp_en);
> +	}
> +
> +	if (ctx->mclk) {

If no-MCLK case even valid here? You're providing new driver for an 
older, specific configuration. Perhaps limiting the code to the actual 
(and only) user is the way to go?

> +		/*
> +		 * The firmware might enable the clock at
> +		 * boot (this information may or may not
> +		 * be reflected in the enable clock register).
> +		 * To change the rate we must disable the clock
> +		 * first to cover these cases. Due to common
> +		 * clock framework restrictions that do not allow
> +		 * to disable a clock that has not been enabled,
> +		 * we need to enable the clock first.
> +		 */
> +		ret = clk_prepare_enable(ctx->mclk);
> +		if (!ret)
> +			clk_disable_unprepare(ctx->mclk);
> +
> +		ret = clk_set_rate(ctx->mclk, CHT_PLAT_CLK_3_HZ);
> +
> +		if (ret) {
> +			dev_err(runtime->dev, "unable to set MCLK rate\n");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}

...

> +#define SOF_CARD_NAME "cht yogabook"
> +#define SOF_DRIVER_NAME "SOF"
> +
> +#define CARD_NAME "cht-yogabook"
> +#define DRIVER_NAME NULL

Have you tested the driver with both, legacy -and- SOF firmware? If 
you're using just one of them, let's limit the driver to that one. 
Anything else can be part of a follow up series if there is a need to 
support multiple solutions. Otherwise we'd be merging code with no 
coverage and no user.

> +
> +static int snd_cht_yb_mc_probe(struct platform_device *pdev)
> +{
> +	int ret_val = 0;
> +	struct cht_yb_private *drv;

No. 'drv', short for 'driver', is not the right name for the private 
context. Typically 'priv' or 'data' is used.

> +	struct snd_soc_acpi_mach *mach = pdev->dev.platform_data;

dev_get_platdata(), no reason to avoid well-established APIs.

> +	const char *platform_name;
> +	struct acpi_device *adev;
> +	struct device *codec_dev;
> +	bool sof_parent;
> +	int i;
> +
> +	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
> +	if (!drv)
> +		return -ENOMEM;
> +
> +	strscpy(drv->codec_name, RT5677_I2C_DEFAULT);
> +
> +	/* fixup codec name based on HID if ACPI node is present */
> +	adev = acpi_dev_get_first_match_dev(mach->id, NULL, -1);
> +	if (adev) {
> +		snprintf(drv->codec_name, sizeof(drv->codec_name),
> +			 "i2c-%s", acpi_dev_name(adev));
> +		dev_info(&pdev->dev, "real codec name: %s\n", drv->codec_name);
> +
> +		put_device(&adev->dev);
> +		for (i = 0; i < ARRAY_SIZE(cht_yb_dailink); i++) {
> +			if (cht_yb_dailink[i].codecs->name &&
> +			    !strcmp(cht_yb_dailink[i].codecs->name,
> +				    RT5677_I2C_DEFAULT)) {
> +				cht_yb_dailink[i].codecs->name = drv->codec_name;
> +				break;
> +			}
> +		}
> +	}
> +
> +	codec_dev = bus_find_device_by_name(&i2c_bus_type, NULL,
> +					    drv->codec_name);
> +	if (!codec_dev)
> +		return -EPROBE_DEFER;
> +
> +	if (adev) {
> +		ret_val = devm_acpi_dev_add_driver_gpios(codec_dev,
> +							 cht_yb_gpios);
> +		if (ret_val)
> +			dev_warn(&pdev->dev,
> +				 "Unable to add GPIO mapping table: %d\n",
> +				 ret_val);
> +	}
> +
> +	/* override platform name, if required */
> +	snd_soc_card_cht_yb.dev = &pdev->dev;
> +	platform_name = mach->mach_params.platform;
> +
> +	ret_val = snd_soc_fixup_dai_links_platform_name(&snd_soc_card_cht_yb,
> +							platform_name);
> +	if (ret_val) {
> +		dev_err(&pdev->dev,
> +			"snd_soc_fixup_dai_links_platform_name failed: %d\n",
> +			ret_val);
> +		return ret_val;
> +	}
> +
> +	drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
> +	if (IS_ERR(drv->mclk)) {
> +		dev_err(&pdev->dev,
> +			"Failed to get MCLK from pmc_plt_clk_3: %ld\n",
> +			PTR_ERR(drv->mclk));
> +		return PTR_ERR(drv->mclk);
> +	}
> +	snd_soc_card_set_drvdata(&snd_soc_card_cht_yb, drv);
> +
> +	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
> +
> +	/* set the card and driver name */
> +	if (sof_parent) {
> +		snd_soc_card_cht_yb.name = SOF_CARD_NAME;
> +		snd_soc_card_cht_yb.driver_name = SOF_DRIVER_NAME;
> +	} else {
> +		snd_soc_card_cht_yb.name = CARD_NAME;
> +		snd_soc_card_cht_yb.driver_name = DRIVER_NAME;
> +	}
> +
> +	/* register the soc card */
> +	snd_soc_card_cht_yb.dev = &pdev->dev;

I'd move away from static-card approach and dynamically allocate the 
object here, in the probe() function.

> +	ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht_yb);
> +	if (ret_val) {
> +		dev_err(&pdev->dev,
> +			"snd_soc_register_card failed %d\n", ret_val);
> +		return ret_val;
> +	}
> +	platform_set_drvdata(pdev, &snd_soc_card_cht_yb);

This seems bogus. The probe() found here is not the function that 
spawned the platform-device 'pdev' yet it attempts to manipulate 
device's private data. At the same time there is no 
platform_get_drvdata() anywhere. Unless there's a strong reason for the 
call, please drop the line.

> +
> +	return ret_val;
> +}
> +
> +static struct platform_driver snd_cht_yb_mc_driver = {
> +	.driver = {
> +		.name = "cht-yogabook",

It seems .pm assignment is missing. I see that the "base" is missing 
this too. Looks like a flaw, I'd assign snd_soc_pm_ops and re-test. The 
ops describe basic bring-up and tear-down procedures and I'm expecting 
them to notify us about any problems rather than harm the configuration.

> +	},
> +	.probe = snd_cht_yb_mc_probe,
> +};
> +
> +module_platform_driver(snd_cht_yb_mc_driver);
> +
> +MODULE_DESCRIPTION("Lenovo Yoga Book YB1-X9x machine driver");
> +MODULE_AUTHOR("Yauhen Kharuzhy");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:cht-yogabook");
> 

Re: [PATCH v2 2/3] ASoC: Intel: soc-acpi-cht: Add Lenovo Yoga Book entries

[Péter Ujfalusi]

On 01/03/2026 23:33, Yauhen Kharuzhy wrote:
> Lenovo Yoga Book YB1-X91 device uses a Cherry Trail SoC and has a valid
> ACPI DSDT entry for the RT5677 codec. This entry has some non-standard
> resource definitions, such as jack detection chip information, and
> hardware has some additional GPIO controls so use 'cht-yogabook'
> for the driver name instead of some default (like 'cht-bsw-rt5677').
> 
> Lenovo Yoga Book YB1-X90 device (Android version of the tablet) has the
> same hardware configuration but lacks a valid ACPI DSDT entry for the
> codec, so add DMI match data for it and use the same machine data as for
> YB1-X91.
> 
> Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
> ---
>  sound/soc/intel/common/soc-acpi-intel-cht-match.c | 40 +++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/sound/soc/intel/common/soc-acpi-intel-cht-match.c b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> index 57097c1d011e..8673ade66e9d 100644
> --- a/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> +++ b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> @@ -17,6 +17,14 @@ static struct snd_soc_acpi_mach cht_surface_mach = {
>  	.sof_tplg_filename = "sof-cht-rt5645.tplg",
>  };
>  
> +static struct snd_soc_acpi_mach cht_yogabook_mach = {
> +	.id = "10EC5677",
> +	.drv_name = "cht-yogabook",
> +	.fw_filename = "intel/fw_sst_22a8.bin",
> +	.board = "cht-yogabook",
> +	.sof_tplg_filename = "sof-cht-rt5677.tplg",
> +};
> +
>  static struct snd_soc_acpi_mach cht_lenovo_yoga_tab3_x90_mach = {
>  	.id = "10WM5102",
>  	.drv_name = "bytcr_wm5102",
> @@ -33,6 +41,24 @@ static const struct dmi_system_id cht_table[] = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
>  		},
>  	},
> +	{
> +		.ident = "Lenovo Yoga Book YB1-X91",
> +		.driver_data = (void *)&cht_yogabook_mach,
> +		/* YB1-X91L/F */
> +		.matches = {
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Lenovo YB1-X91"),
> +		}
> +	},
> +	{
> +		.ident = "Lenovo Yoga Book YB1-X90",
> +		.driver_data = (void *)&cht_yogabook_mach,
> +		/* YB1-X90L/F, codec is not listed in DSDT */
> +		.matches = {
> +			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Intel Corporation"),
> +			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "CHERRYVIEW D1 PLATFORM"),
> +			DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "YETI-11"),
> +		}
> +	},
>  	{
>  		/*
>  		 * The Lenovo Yoga Tab 3 Pro YT3-X90, with Android factory OS
> @@ -121,6 +147,20 @@ struct snd_soc_acpi_mach  snd_soc_acpi_intel_cherrytrail_machines[] = {
>  		.board = "cht-bsw",
>  		.sof_tplg_filename = "sof-cht-rt5670.tplg",
>  	},
> +	/*
> +	 * The only known Cherry Trail device with RT5677 codec and 10EC677
> +	 * DSTD entry is the Lenovo Yoga Book YB1-X91. It has a device-specific
> +	 * driver, so check DMI and use a machine quirk to override the default
> +	 * (non-existent) machine driver.
> +	 */
> +	{
> +		.id = "10EC5677",
> +		.drv_name = "cht-bsw-rt5677",

there is no such driver exists

> +		.fw_filename = "intel/fw_sst_22a8.bin",
> +		.board = "cht-bsw",
> +		.machine_quirk = cht_quirk,
> +		.sof_tplg_filename = "sof-cht-rt5677.tplg",

There is no such topology exists

> +	},
>  	{
>  		.comp_ids = &rt5645_comp_ids,
>  		.drv_name = "cht-bsw-rt5645",
> 

-- 
Péter

Re: [PATCH v2 1/3] ASoC: Intel: soc-acpi-cht: Unify device quirks

[Péter Ujfalusi]

On 01/03/2026 23:33, Yauhen Kharuzhy wrote:
> This file contains two types of quirks, both checking DMI for
> machine-specific strings and returning machine data for a matching entry.
> 
> The first one, `cht_quirk`, is used to override the default entry for an
> existing ACPI codec node if the node's info is invalid. It returns either
> the matched machine data or the default entry if no match is found.
> 
> The second one, `cht_yt3_quirk_cb`, is used for devices (originally the
> Lenovo Yoga Tab 3 Pro) without a valid codec DSDT entry. It is bound to
> the SST ACPI node and returns either the matched machine data or NULL if
> no match is found.
> 
> To allow adding new machine entries to the second case and to use a single
> DMI match entry for both cases (for example, if two variants of one device
> exist: one with a valid ACPI entry and one without, like the Lenovo Yoga
> Book YB1-X91 and YB1-X90 - Windows and Android versions), reorganize
> these quirks functions to use the same approach: machine data is set in
> the matched dmi_system_id entry as driver_data field.
> 
> Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
> ---
>  sound/soc/intel/common/soc-acpi-intel-cht-match.c | 100 +++++++++-------------
>  1 file changed, 42 insertions(+), 58 deletions(-)
> 
> diff --git a/sound/soc/intel/common/soc-acpi-intel-cht-match.c b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> index e4c3492a0c28..57097c1d011e 100644
> --- a/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> +++ b/sound/soc/intel/common/soc-acpi-intel-cht-match.c

>  
> +static struct snd_soc_acpi_mach *cht_quirk_nocodec(void *arg)

This is confusing, why it is _nocodec?
cht_quirk_strict() or something might be better?

-- 
Péter

Re: [PATCH v2 1/3] ASoC: Intel: soc-acpi-cht: Unify device quirks

[Yauhen Kharuzhy]

On Mon, Mar 02, 2026 at 05:54:05PM +0200, Péter Ujfalusi wrote:
> 
> 
> On 01/03/2026 23:33, Yauhen Kharuzhy wrote:
> > This file contains two types of quirks, both checking DMI for
> > machine-specific strings and returning machine data for a matching entry.
> > 
> > The first one, `cht_quirk`, is used to override the default entry for an
> > existing ACPI codec node if the node's info is invalid. It returns either
> > the matched machine data or the default entry if no match is found.
> > 
> > The second one, `cht_yt3_quirk_cb`, is used for devices (originally the
> > Lenovo Yoga Tab 3 Pro) without a valid codec DSDT entry. It is bound to
> > the SST ACPI node and returns either the matched machine data or NULL if
> > no match is found.
> > 
> > To allow adding new machine entries to the second case and to use a single
> > DMI match entry for both cases (for example, if two variants of one device
> > exist: one with a valid ACPI entry and one without, like the Lenovo Yoga
> > Book YB1-X91 and YB1-X90 - Windows and Android versions), reorganize
> > these quirks functions to use the same approach: machine data is set in
> > the matched dmi_system_id entry as driver_data field.
> > 
> > Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
> > ---
> >  sound/soc/intel/common/soc-acpi-intel-cht-match.c | 100 +++++++++-------------
> >  1 file changed, 42 insertions(+), 58 deletions(-)
> > 
> > diff --git a/sound/soc/intel/common/soc-acpi-intel-cht-match.c b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> > index e4c3492a0c28..57097c1d011e 100644
> > --- a/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> > +++ b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> 
> >  
> > +static struct snd_soc_acpi_mach *cht_quirk_nocodec(void *arg)
> 
> This is confusing, why it is _nocodec?
> cht_quirk_strict() or something might be better?

Something like "a quirk for machines without of codec definition in the
ACPI DSDT". But yes, it is confusing because we have separated "nocodec" entry
in the table. I just couldn't think of anything better. 'strict' doesn't
seem to reflect the meaning for my opinion also.


> 
> -- 
> Péter
> 

-- 
Yauhen Kharuzhy

Re: [PATCH v2 3/3] ASoC: Intel: cht_yogabook: Add driver for Lenovo Yoga Book tablets

[Yauhen Kharuzhy]

On Mon, Mar 02, 2026 at 01:03:04PM +0100, Cezary Rojewski wrote:
> On 2026-03-01 10:33 PM, Yauhen Kharuzhy wrote:
> > Add a new ASoC machine driver for Lenovo Yoga Book Intel Cherry
> > Trail-based tablets (YB1-X90F/L, YB1-X91F/L models).
> > 
> > This platform uses a Realtek ALC5677 codec accompanied by a TS3A227E jack
> > configuration detection IC.
> > 
> > The driver is based on the cht_bsw_rt5672.c mainline driver and the driver
> > from the vendor's Android kernel [1].
> > 
> > There are no other known Cherry Trail platforms using an RT5677 codec, so
> > the driver is named 'cht_yogabook', and some device-specific tricks are
> > hardcoded, such as jack events and additional GPIOs controlling the
> > speaker amplifier.
> 
> I'd switch to more specific naming. From the Intel-maintainer point of view,
> it's easier to navigate between configurations when <platform>-<codec>
> naming is in use. TBH, we do not see "yogabook", we configuration composed
> of Cherrytrail-based device and rt5677 I2C codec.

So, should it just be named "cht-rt5677", but the code inside will be Yoga
Book-specific without generalization? Or is it better to make the code as
generic as possible and add machine-specific things via some kind of quirks
selected by DMI data, for example? I doubt if we will meet another
working Cherry Trail-based device with RT5677 codec in the future.

> 
> For the entire driver:
> - fix alignments

There are alignment rule violations only at SND_SOC_DAILINK_DEF()
declarations where they looks acceptable. I just copied their style from
other intel board drivers.

But OK, I will change this.

> - fix char-per-line limit, submission rules expect 100c-per-line limit

checkpatch.pl reports no such violations except of the link in the commit
description.


> 
> Below additional comments.
> 
> > 
> > [1] https://github.com/deadman96385/android_kernel_lenovo_yeti/blob/master/sound/soc/intel/board/cht_bl_dpcm_rt5677.c
> > 
> > Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
> 
> ...
> 
> 
> > diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
> > index 25a1a9066cbf..58bd6029545b 100644
> > --- a/sound/soc/intel/boards/Makefile
> > +++ b/sound/soc/intel/boards/Makefile
> > @@ -15,6 +15,7 @@ snd-soc-sst-cht-bsw-nau8824-y := cht_bsw_nau8824.o
> >   snd-soc-sst-byt-cht-cx2072x-y := bytcht_cx2072x.o
> >   snd-soc-sst-byt-cht-da7213-y := bytcht_da7213.o
> >   snd-soc-sst-byt-cht-es8316-y := bytcht_es8316.o
> > +snd-soc-sst-cht-yogabook-objs := cht_yogabook.o
> 
> Let's follow recommendation from Takashi [1]. Suffix "-objs" is obsolete.
> 
> [1]: https://lore.kernel.org/all/20240507155540.24815-13-tiwai@suse.de/

OK.

> 
> >   snd-soc-sst-byt-cht-nocodec-y := bytcht_nocodec.o
> >   snd-soc-sof_rt5682-y := sof_rt5682.o
> >   snd-soc-sof_cs42l42-y := sof_cs42l42.o
> > @@ -47,6 +48,7 @@ obj-$(CONFIG_SND_SOC_INTEL_CHT_BSW_NAU8824_MACH) += snd-soc-sst-cht-bsw-nau8824.
> >   obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_CX2072X_MACH) += snd-soc-sst-byt-cht-cx2072x.o
> >   obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_DA7213_MACH) += snd-soc-sst-byt-cht-da7213.o
> >   obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_ES8316_MACH) += snd-soc-sst-byt-cht-es8316.o
> > +obj-$(CONFIG_SND_SOC_INTEL_CHT_YOGABOOK_MACH) += snd-soc-sst-cht-yogabook.o
> >   obj-$(CONFIG_SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH) += snd-soc-sst-byt-cht-nocodec.o
> >   obj-$(CONFIG_SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH) += snd-soc-skl_hda_dsp.o
> >   obj-$(CONFIG_SND_SOC_INTEL_EHL_RT5660_MACH) += snd-soc-ehl-rt5660.o
> > diff --git a/sound/soc/intel/boards/cht_yogabook.c b/sound/soc/intel/boards/cht_yogabook.c
> > new file mode 100644
> > index 000000000000..9d945cad5660
> > --- /dev/null
> > +++ b/sound/soc/intel/boards/cht_yogabook.c
> > @@ -0,0 +1,551 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + *  cht_yogabook.c - ASoc Machine driver for Lenovo Yoga Book YB1-X90/X91
> > + *  tablets, based on Intel Cherrytrail platform with RT5677 codec.
> > + *
> > + *  Copyright (C) 2026 Yauhen Kharuzhy <jekhor@gmail.com>
> > + *
> > + *  Based on cht_bsw_rt5672.c:
> > + *  Copyright (C) 2014 Intel Corp
> > + *  Author: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> > + *          Mengdong Lin <mengdong.lin@intel.com>
> > + *
> > + *  And based on the cht_bl_dpcm_rt5677.c from the Lenovo's Android kernel:
> > + *  Copyright (C) 2014 Intel Corp
> > + *  Author: Mythri P K <mythri.p.k@intel.com>
> 
> Not sure about the boiler plate here. Mentioning people is OK but the emails
> are all obsolete. Such information does not help anyone. Perhaps "based on
> XYZ" is enough.

OK

> 
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/dmi.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/gpio/machine.h>
> > +#include <linux/i2c.h>
> > +#include <linux/input.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +#include <sound/jack.h>
> > +#include <sound/pcm.h>
> > +#include <sound/pcm_params.h>
> > +#include <sound/soc-acpi.h>
> > +#include <sound/soc.h>
> > +#include "../../codecs/rt5677.h"
> > +#include "../../codecs/ts3a227e.h"
> > +#include "../atom/sst-atom-controls.h"
> > +
> > +#define RT5677_I2C_DEFAULT	"i2c-rt5677"
> > +
> > +/* The platform clock #3 outputs 19.2Mhz clock to codec as I2S MCLK */
> > +#define CHT_PLAT_CLK_3_HZ	19200000
> > +#define CHT_CODEC_DAI		"rt5677-aif1"
> > +
> > +struct cht_yb_private {
> > +	char codec_name[SND_ACPI_I2C_ID_LEN];
> > +	struct snd_soc_jack jack;
> > +	struct clk *mclk;
> > +	struct gpio_desc *gpio_spk_en1;
> > +	struct gpio_desc *gpio_spk_en2;
> > +	struct gpio_desc *gpio_hp_en;
> > +};
> > +
> > +static int cht_yb_platform_clock_control(struct snd_soc_dapm_widget *w,
> > +					 struct snd_kcontrol *k, int  event)
> 
> Please replace 'k' with 'kctl' for the entire file.

OK.

> > +{
> > +	struct snd_soc_card *card = snd_soc_dapm_to_card(w->dapm);
> > +	struct snd_soc_dai *codec_dai;
> > +	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(card);
> > +	int ret;
> > +
> > +	codec_dai = snd_soc_card_get_codec_dai(card, CHT_CODEC_DAI);
> > +	if (!codec_dai) {
> > +		dev_err(card->dev,
> > +			"Codec dai not found; Unable to set platform clock\n");
> > +		return -EIO;
> > +	}
> > +
> > +	if (SND_SOC_DAPM_EVENT_ON(event)) {
> > +		if (ctx->mclk) {
> > +			ret = clk_prepare_enable(ctx->mclk);
> > +			if (ret < 0) {
> > +				dev_err(card->dev,
> > +					"could not configure MCLK state");
> > +				return ret;
> > +			}
> > +		}
> > +
> > +		/* set codec PLL source to the 19.2MHz platform clock (MCLK) */
> > +		ret = snd_soc_dai_set_pll(codec_dai, 0, RT5677_PLL1_S_MCLK,
> > +					  CHT_PLAT_CLK_3_HZ, 48000 * 512);
> > +		if (ret < 0) {
> > +			dev_err(card->dev, "can't set codec pll: %d\n", ret);
> > +			return ret;
> > +		}
> > +
> > +		/* set codec sysclk source to PLL */
> > +		ret = snd_soc_dai_set_sysclk(codec_dai, RT5677_SCLK_S_PLL1,
> > +					     48000 * 512, SND_SOC_CLOCK_IN);
> > +		if (ret < 0) {
> > +			dev_err(card->dev, "can't set codec sysclk: %d\n", ret);
> > +			return ret;
> > +		}
> > +	} else {
> > +		/* Set codec sysclk source to its internal clock because codec
> > +		 * PLL will be off when idle and MCLK will also be off by ACPI
> > +		 * when codec is runtime suspended. Codec needs clock for jack
> > +		 * detection and button press.
> > +		 */
> > +		snd_soc_dai_set_sysclk(codec_dai, RT5677_SCLK_S_RCCLK,
> > +				       48000 * 512, SND_SOC_CLOCK_IN);
> > +
> > +		if (ctx->mclk)
> > +			clk_disable_unprepare(ctx->mclk);
> > +	}
> 
> The entire function is made of if-statement. I'd split this into:
> 
> xyz_platform_clock_control()
> |_ xyz_platform_clock_enable()
> |_ xyz_platform_clock_disable()
> 
> 

OK

> > +	return 0;
> > +}
> > +
> > +static int cht_yb_hp_event(struct snd_soc_dapm_widget *w,
> > +			   struct snd_kcontrol *k, int event)
> > +{
> > +	struct snd_soc_card *card = snd_soc_dapm_to_card(w->dapm);
> > +	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(card);
> > +
> > +	dev_dbg(card->dev, "HP event: %s\n",
> > +		SND_SOC_DAPM_EVENT_ON(event) ? "ON" : "OFF");
> 
> These dev_dbg() can be dropped. Function tracer or DAPM events found within
> tracing/events are sufficient.

OK

> 
> > +
> > +	gpiod_set_value_cansleep(ctx->gpio_hp_en, SND_SOC_DAPM_EVENT_ON(event));
> > +
> > +	return 0;
> > +}
> > +
> > +static int cht_yb_spk_event(struct snd_soc_dapm_widget *w,
> > +			    struct snd_kcontrol *k, int event)
> > +{
> > +	struct snd_soc_card *card = snd_soc_dapm_to_card(w->dapm);
> > +	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(card);
> > +
> > +	dev_dbg(card->dev, "SPK event: %s\n",
> > +		SND_SOC_DAPM_EVENT_ON(event) ? "ON" : "OFF");
> 
> Ditto.
> 
> > +
> > +	gpiod_set_value_cansleep(ctx->gpio_spk_en1,
> > +				 SND_SOC_DAPM_EVENT_ON(event));
> > +	gpiod_set_value_cansleep(ctx->gpio_spk_en2,
> > +				 SND_SOC_DAPM_EVENT_ON(event));
> > +
> > +	return 0;
> > +}
> 
> ...
> 
> > +static int cht_yb_codec_init(struct snd_soc_pcm_runtime *runtime)
> > +{
> > +	int ret = 0;
> 
> Move as last.

OK

> 
> > +	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(runtime, 0);
> > +	struct snd_soc_component *component = codec_dai->component;
> > +	struct cht_yb_private *ctx = snd_soc_card_get_drvdata(runtime->card);
> > +
> > +	/* Enable codec ASRC function for Stereo DAC/Stereo1 ADC/DMIC/I2S1.
> > +	 * The ASRC clock source is clk_i2s1_asrc.
> > +	 */
> > +	rt5677_sel_asrc_clk_src(component, RT5677_DA_STEREO_FILTER |
> > +			RT5677_AD_STEREO1_FILTER | RT5677_I2S1_SOURCE,
> > +			RT5677_CLK_SEL_I2S1_ASRC);
> > +	/* Enable codec ASRC function for Mono ADC L.
> > +	 * The ASRC clock source is clk_sys2_asrc.
> > +	 */
> > +	rt5677_sel_asrc_clk_src(component, RT5677_AD_MONO_L_FILTER,
> > +				RT5677_CLK_SEL_SYS2);
> > +
> > +	ctx->gpio_spk_en1 = devm_gpiod_get(component->dev, "speaker-enable",
> > +					   GPIOD_OUT_LOW);
> > +	if (IS_ERR(ctx->gpio_spk_en1)) {
> > +		dev_err(component->dev, "Can't find speaker enable GPIO\n");
> > +		return PTR_ERR(ctx->gpio_spk_en1);
> > +	}
> > +
> > +	ctx->gpio_spk_en2 = devm_gpiod_get(component->dev, "speaker-enable2",
> > +					   GPIOD_OUT_LOW);
> > +	if (IS_ERR(ctx->gpio_spk_en2)) {
> > +		dev_err(component->dev, "Can't find speaker enable 2 GPIO\n");
> > +		return PTR_ERR(ctx->gpio_spk_en2);
> > +	}
> > +
> > +	ctx->gpio_hp_en = devm_gpiod_get(component->dev, "headphone-enable",
> > +					 GPIOD_OUT_LOW);
> > +	if (IS_ERR(ctx->gpio_hp_en)) {
> > +		dev_err(component->dev, "Can't find headphone enable GPIO\n");
> 
> The messages here are misleading. "Can't find ..." when devm_gpiod_get()
> returns different IS_ERR() than -ENOENT is incorrect. I'd just state that
> XYZ operation failed and append the return code. Scales nicely into the
> future.

ok

> 
> > +		return PTR_ERR(ctx->gpio_hp_en);
> > +	}
> > +
> > +	if (ctx->mclk) {
> 
> If no-MCLK case even valid here? You're providing new driver for an older,
> specific configuration. Perhaps limiting the code to the actual (and only)
> user is the way to go?

Yes, you are correct, the driver fails to probe if the mclk is not obtained.
Will remove such checks.

> 
> > +		/*
> > +		 * The firmware might enable the clock at
> > +		 * boot (this information may or may not
> > +		 * be reflected in the enable clock register).
> > +		 * To change the rate we must disable the clock
> > +		 * first to cover these cases. Due to common
> > +		 * clock framework restrictions that do not allow
> > +		 * to disable a clock that has not been enabled,
> > +		 * we need to enable the clock first.
> > +		 */
> > +		ret = clk_prepare_enable(ctx->mclk);
> > +		if (!ret)
> > +			clk_disable_unprepare(ctx->mclk);
> > +
> > +		ret = clk_set_rate(ctx->mclk, CHT_PLAT_CLK_3_HZ);
> > +
> > +		if (ret) {
> > +			dev_err(runtime->dev, "unable to set MCLK rate\n");
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> 
> ...
> 
> > +#define SOF_CARD_NAME "cht yogabook"
> > +#define SOF_DRIVER_NAME "SOF"
> > +
> > +#define CARD_NAME "cht-yogabook"
> > +#define DRIVER_NAME NULL
> 
> Have you tested the driver with both, legacy -and- SOF firmware? If you're
> using just one of them, let's limit the driver to that one. Anything else
> can be part of a follow up series if there is a need to support multiple
> solutions. Otherwise we'd be merging code with no coverage and no user.

No, I tested only with non-SOF firmware because topology file for rt5677
is missing. Agree, I will remove SOF related code.

> 
> > +
> > +static int snd_cht_yb_mc_probe(struct platform_device *pdev)
> > +{
> > +	int ret_val = 0;
> > +	struct cht_yb_private *drv;
> 
> No. 'drv', short for 'driver', is not the right name for the private
> context. Typically 'priv' or 'data' is used.

OK

> 
> > +	struct snd_soc_acpi_mach *mach = pdev->dev.platform_data;
> 
> dev_get_platdata(), no reason to avoid well-established APIs.
OK

> 
> > +	const char *platform_name;
> > +	struct acpi_device *adev;
> > +	struct device *codec_dev;
> > +	bool sof_parent;
> > +	int i;
> > +
> > +	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
> > +	if (!drv)
> > +		return -ENOMEM;
> > +
> > +	strscpy(drv->codec_name, RT5677_I2C_DEFAULT);
> > +
> > +	/* fixup codec name based on HID if ACPI node is present */
> > +	adev = acpi_dev_get_first_match_dev(mach->id, NULL, -1);
> > +	if (adev) {
> > +		snprintf(drv->codec_name, sizeof(drv->codec_name),
> > +			 "i2c-%s", acpi_dev_name(adev));
> > +		dev_info(&pdev->dev, "real codec name: %s\n", drv->codec_name);
> > +
> > +		put_device(&adev->dev);
> > +		for (i = 0; i < ARRAY_SIZE(cht_yb_dailink); i++) {
> > +			if (cht_yb_dailink[i].codecs->name &&
> > +			    !strcmp(cht_yb_dailink[i].codecs->name,
> > +				    RT5677_I2C_DEFAULT)) {
> > +				cht_yb_dailink[i].codecs->name = drv->codec_name;
> > +				break;
> > +			}
> > +		}
> > +	}
> > +
> > +	codec_dev = bus_find_device_by_name(&i2c_bus_type, NULL,
> > +					    drv->codec_name);
> > +	if (!codec_dev)
> > +		return -EPROBE_DEFER;
> > +
> > +	if (adev) {
> > +		ret_val = devm_acpi_dev_add_driver_gpios(codec_dev,
> > +							 cht_yb_gpios);
> > +		if (ret_val)
> > +			dev_warn(&pdev->dev,
> > +				 "Unable to add GPIO mapping table: %d\n",
> > +				 ret_val);
> > +	}
> > +
> > +	/* override platform name, if required */
> > +	snd_soc_card_cht_yb.dev = &pdev->dev;
> > +	platform_name = mach->mach_params.platform;
> > +
> > +	ret_val = snd_soc_fixup_dai_links_platform_name(&snd_soc_card_cht_yb,
> > +							platform_name);
> > +	if (ret_val) {
> > +		dev_err(&pdev->dev,
> > +			"snd_soc_fixup_dai_links_platform_name failed: %d\n",
> > +			ret_val);
> > +		return ret_val;
> > +	}
> > +
> > +	drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
> > +	if (IS_ERR(drv->mclk)) {
> > +		dev_err(&pdev->dev,
> > +			"Failed to get MCLK from pmc_plt_clk_3: %ld\n",
> > +			PTR_ERR(drv->mclk));
> > +		return PTR_ERR(drv->mclk);
> > +	}
> > +	snd_soc_card_set_drvdata(&snd_soc_card_cht_yb, drv);
> > +
> > +	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
> > +
> > +	/* set the card and driver name */
> > +	if (sof_parent) {
> > +		snd_soc_card_cht_yb.name = SOF_CARD_NAME;
> > +		snd_soc_card_cht_yb.driver_name = SOF_DRIVER_NAME;
> > +	} else {
> > +		snd_soc_card_cht_yb.name = CARD_NAME;
> > +		snd_soc_card_cht_yb.driver_name = DRIVER_NAME;
> > +	}
> > +
> > +	/* register the soc card */
> > +	snd_soc_card_cht_yb.dev = &pdev->dev;
> 
> I'd move away from static-card approach and dynamically allocate the object
> here, in the probe() function.

hmm... OK but why? Most sound drivers use a static approach to simplify
declaration.

> 
> > +	ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht_yb);
> > +	if (ret_val) {
> > +		dev_err(&pdev->dev,
> > +			"snd_soc_register_card failed %d\n", ret_val);
> > +		return ret_val;
> > +	}
> > +	platform_set_drvdata(pdev, &snd_soc_card_cht_yb);
> 
> This seems bogus. The probe() found here is not the function that spawned
> the platform-device 'pdev' yet it attempts to manipulate device's private
> data. At the same time there is no platform_get_drvdata() anywhere. Unless
> there's a strong reason for the call, please drop the line.

Sure, snaked from the cht_bsw_rt5672.c

> 
> > +
> > +	return ret_val;
> > +}
> > +
> > +static struct platform_driver snd_cht_yb_mc_driver = {
> > +	.driver = {
> > +		.name = "cht-yogabook",
> 
> It seems .pm assignment is missing. I see that the "base" is missing this
> too. Looks like a flaw, I'd assign snd_soc_pm_ops and re-test. The ops
> describe basic bring-up and tear-down procedures and I'm expecting them to
> notify us about any problems rather than harm the configuration.

OK

> 
> > +	},
> > +	.probe = snd_cht_yb_mc_probe,
> > +};
> > +
> > +module_platform_driver(snd_cht_yb_mc_driver);
> > +
> > +MODULE_DESCRIPTION("Lenovo Yoga Book YB1-X9x machine driver");
> > +MODULE_AUTHOR("Yauhen Kharuzhy");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_ALIAS("platform:cht-yogabook");
> > 
> 

-- 
Yauhen Kharuzhy

Re: [PATCH v2 1/3] ASoC: Intel: soc-acpi-cht: Unify device quirks

[Péter Ujfalusi]

On 03/03/2026 00:33, Yauhen Kharuzhy wrote:
> On Mon, Mar 02, 2026 at 05:54:05PM +0200, Péter Ujfalusi wrote:
>>
>>
>> On 01/03/2026 23:33, Yauhen Kharuzhy wrote:
>>> This file contains two types of quirks, both checking DMI for
>>> machine-specific strings and returning machine data for a matching entry.
>>>
>>> The first one, `cht_quirk`, is used to override the default entry for an
>>> existing ACPI codec node if the node's info is invalid. It returns either
>>> the matched machine data or the default entry if no match is found.
>>>
>>> The second one, `cht_yt3_quirk_cb`, is used for devices (originally the
>>> Lenovo Yoga Tab 3 Pro) without a valid codec DSDT entry. It is bound to
>>> the SST ACPI node and returns either the matched machine data or NULL if
>>> no match is found.
>>>
>>> To allow adding new machine entries to the second case and to use a single
>>> DMI match entry for both cases (for example, if two variants of one device
>>> exist: one with a valid ACPI entry and one without, like the Lenovo Yoga
>>> Book YB1-X91 and YB1-X90 - Windows and Android versions), reorganize
>>> these quirks functions to use the same approach: machine data is set in
>>> the matched dmi_system_id entry as driver_data field.
>>>
>>> Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
>>> ---
>>>  sound/soc/intel/common/soc-acpi-intel-cht-match.c | 100 +++++++++-------------
>>>  1 file changed, 42 insertions(+), 58 deletions(-)
>>>
>>> diff --git a/sound/soc/intel/common/soc-acpi-intel-cht-match.c b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
>>> index e4c3492a0c28..57097c1d011e 100644
>>> --- a/sound/soc/intel/common/soc-acpi-intel-cht-match.c
>>> +++ b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
>>
>>>  
>>> +static struct snd_soc_acpi_mach *cht_quirk_nocodec(void *arg)
>>
>> This is confusing, why it is _nocodec?
>> cht_quirk_strict() or something might be better?
> 
> Something like "a quirk for machines without of codec definition in the
> ACPI DSDT". But yes, it is confusing because we have separated "nocodec" entry
> in the table. I just couldn't think of anything better. 'strict' doesn't
> seem to reflect the meaning for my opinion also.

I see, the strict came to mind to imply that it will only return with a
match if there is a match, otherwise it will return NULL.
No fallback to default (or self match), only a strict match is allowed.

cht_match_only_quirk(), cht_no_fallback_quirk() ?

I'm extremely bad at naming..

> 
> 
>>
>> -- 
>> Péter
>>
> 

-- 
Péter

Re: [PATCH v2 3/3] ASoC: Intel: cht_yogabook: Add driver for Lenovo Yoga Book tablets

[Cezary Rojewski]

On 2026-03-03 1:12 AM, Yauhen Kharuzhy wrote:
> On Mon, Mar 02, 2026 at 01:03:04PM +0100, Cezary Rojewski wrote:
>> On 2026-03-01 10:33 PM, Yauhen Kharuzhy wrote:
>>> Add a new ASoC machine driver for Lenovo Yoga Book Intel Cherry
>>> Trail-based tablets (YB1-X90F/L, YB1-X91F/L models).
>>>
>>> This platform uses a Realtek ALC5677 codec accompanied by a TS3A227E jack
>>> configuration detection IC.
>>>
>>> The driver is based on the cht_bsw_rt5672.c mainline driver and the driver
>>> from the vendor's Android kernel [1].
>>>
>>> There are no other known Cherry Trail platforms using an RT5677 codec, so
>>> the driver is named 'cht_yogabook', and some device-specific tricks are
>>> hardcoded, such as jack events and additional GPIOs controlling the
>>> speaker amplifier.
>>
>> I'd switch to more specific naming. From the Intel-maintainer point of view,
>> it's easier to navigate between configurations when <platform>-<codec>
>> naming is in use. TBH, we do not see "yogabook", we configuration composed
>> of Cherrytrail-based device and rt5677 I2C codec.
> 
> So, should it just be named "cht-rt5677", but the code inside will be Yoga
> Book-specific without generalization? Or is it better to make the code as
> generic as possible and add machine-specific things via some kind of quirks
> selected by DMI data, for example? I doubt if we will meet another
> working Cherry Trail-based device with RT5677 codec in the future.

"cht_rt5677" sounds like a good filename for this very driver. Yeah, I 
should have used '_' when mentioning <platform>_<codec> in my previous 
response, so that the naming remains cohesive with the vast majority. Oh 
well..

In regard to the generic/specific - I typically turn to the coverage and 
the schedule to answer such question. I believe the specific tablets you 
mentioned are the only coverage for the driver. And thus I do not see a 
reason to scale beyond that.

The generic approach is good (and usually favoured) when one provides a 
feature or a driver for specific configuration XYZ_v1 but with XYZ_v2, 
that's coming a year from now, already in mind. I do not think this is 
the case either.

>>
>> For the entire driver:
>> - fix alignments
> 
> There are alignment rule violations only at SND_SOC_DAILINK_DEF()
> declarations where they looks acceptable. I just copied their style from
> other intel board drivers.
> 
> But OK, I will change this.
> 
>> - fix char-per-line limit, submission rules expect 100c-per-line limit
> 
> checkpatch.pl reports no such violations except of the link in the commit
> description.

Few years ago the limit was raised and basically anything we do for 
intel-sound utilizes that limit. checkpatch warns about violations, 80 
or 100 is more of a preference. So, nothing to worry about, just a 
request to align with what we provide daily.

...

>>> +	const char *platform_name;
>>> +	struct acpi_device *adev;
>>> +	struct device *codec_dev;
>>> +	bool sof_parent;
>>> +	int i;
>>> +
>>> +	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
>>> +	if (!drv)
>>> +		return -ENOMEM;
>>> +
>>> +	strscpy(drv->codec_name, RT5677_I2C_DEFAULT);
>>> +
>>> +	/* fixup codec name based on HID if ACPI node is present */
>>> +	adev = acpi_dev_get_first_match_dev(mach->id, NULL, -1);
>>> +	if (adev) {
>>> +		snprintf(drv->codec_name, sizeof(drv->codec_name),
>>> +			 "i2c-%s", acpi_dev_name(adev));
>>> +		dev_info(&pdev->dev, "real codec name: %s\n", drv->codec_name);
>>> +
>>> +		put_device(&adev->dev);
>>> +		for (i = 0; i < ARRAY_SIZE(cht_yb_dailink); i++) {
>>> +			if (cht_yb_dailink[i].codecs->name &&
>>> +			    !strcmp(cht_yb_dailink[i].codecs->name,
>>> +				    RT5677_I2C_DEFAULT)) {
>>> +				cht_yb_dailink[i].codecs->name = drv->codec_name;
>>> +				break;
>>> +			}
>>> +		}
>>> +	}
>>> +
>>> +	codec_dev = bus_find_device_by_name(&i2c_bus_type, NULL,
>>> +					    drv->codec_name);
>>> +	if (!codec_dev)
>>> +		return -EPROBE_DEFER;
>>> +
>>> +	if (adev) {
>>> +		ret_val = devm_acpi_dev_add_driver_gpios(codec_dev,
>>> +							 cht_yb_gpios);
>>> +		if (ret_val)
>>> +			dev_warn(&pdev->dev,
>>> +				 "Unable to add GPIO mapping table: %d\n",
>>> +				 ret_val);
>>> +	}
>>> +
>>> +	/* override platform name, if required */
>>> +	snd_soc_card_cht_yb.dev = &pdev->dev;
>>> +	platform_name = mach->mach_params.platform;
>>> +
>>> +	ret_val = snd_soc_fixup_dai_links_platform_name(&snd_soc_card_cht_yb,
>>> +							platform_name);
>>> +	if (ret_val) {
>>> +		dev_err(&pdev->dev,
>>> +			"snd_soc_fixup_dai_links_platform_name failed: %d\n",
>>> +			ret_val);
>>> +		return ret_val;
>>> +	}
>>> +
>>> +	drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
>>> +	if (IS_ERR(drv->mclk)) {
>>> +		dev_err(&pdev->dev,
>>> +			"Failed to get MCLK from pmc_plt_clk_3: %ld\n",
>>> +			PTR_ERR(drv->mclk));
>>> +		return PTR_ERR(drv->mclk);
>>> +	}
>>> +	snd_soc_card_set_drvdata(&snd_soc_card_cht_yb, drv);
>>> +
>>> +	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
>>> +
>>> +	/* set the card and driver name */
>>> +	if (sof_parent) {
>>> +		snd_soc_card_cht_yb.name = SOF_CARD_NAME;
>>> +		snd_soc_card_cht_yb.driver_name = SOF_DRIVER_NAME;
>>> +	} else {
>>> +		snd_soc_card_cht_yb.name = CARD_NAME;
>>> +		snd_soc_card_cht_yb.driver_name = DRIVER_NAME;
>>> +	}
>>> +
>>> +	/* register the soc card */
>>> +	snd_soc_card_cht_yb.dev = &pdev->dev;
>>
>> I'd move away from static-card approach and dynamically allocate the object
>> here, in the probe() function.
> 
> hmm... OK but why? Most sound drivers use a static approach to simplify
> declaration.

Most of the "legacy" machine-board drivers I'd say. If you take a look 
at SOF's or avs's (intel/avs/boards) the number of static cards is limited.

There is no _strong_ argument against or in favour of either. The reason 
I suggest to switch to the dynamic approach is similar to what you do 
here - base your code on someone else' work. When such copy lands on a 
configuration where not one but two I2C codecs are present, more than 
one sound card might be desired.
With statics, things would get messy in runtime. With dynamic allocation 
developer can forget about problems related to inheriting some unwanted 
data.

Small bonus is not occupying space for the card object until the 
platform-device (that represents the sound card) is ready for probing. 
The static descriptor occupies space the moment the module is launched.

>>
>>> +	ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht_yb);
>>> +	if (ret_val) {
>>> +		dev_err(&pdev->dev,
>>> +			"snd_soc_register_card failed %d\n", ret_val);
>>> +		return ret_val;
>>> +	}
>>> +	platform_set_drvdata(pdev, &snd_soc_card_cht_yb);
>>
>> This seems bogus. The probe() found here is not the function that spawned
>> the platform-device 'pdev' yet it attempts to manipulate device's private
>> data. At the same time there is no platform_get_drvdata() anywhere. Unless
>> there's a strong reason for the call, please drop the line.
> 
> Sure, snaked from the cht_bsw_rt5672.c

These older machine-boards drivers contain some bad examples, unfortunately.

Re: [PATCH v2 3/3] ASoC: Intel: cht_yogabook: Add driver for Lenovo Yoga Book tablets

[Yauhen Kharuzhy]

On Tue, Mar 03, 2026 at 10:30:22AM +0100, Cezary Rojewski wrote:
> On 2026-03-03 1:12 AM, Yauhen Kharuzhy wrote:
> > On Mon, Mar 02, 2026 at 01:03:04PM +0100, Cezary Rojewski wrote:
> > > On 2026-03-01 10:33 PM, Yauhen Kharuzhy wrote:
> > > > Add a new ASoC machine driver for Lenovo Yoga Book Intel Cherry
> > > > Trail-based tablets (YB1-X90F/L, YB1-X91F/L models).
> > > > 
> > > > This platform uses a Realtek ALC5677 codec accompanied by a TS3A227E jack
> > > > configuration detection IC.
> > > > 
> > > > The driver is based on the cht_bsw_rt5672.c mainline driver and the driver
> > > > from the vendor's Android kernel [1].
> > > > 
> > > > There are no other known Cherry Trail platforms using an RT5677 codec, so
> > > > the driver is named 'cht_yogabook', and some device-specific tricks are
> > > > hardcoded, such as jack events and additional GPIOs controlling the
> > > > speaker amplifier.
> > > 
> > > I'd switch to more specific naming. From the Intel-maintainer point of view,
> > > it's easier to navigate between configurations when <platform>-<codec>
> > > naming is in use. TBH, we do not see "yogabook", we configuration composed
> > > of Cherrytrail-based device and rt5677 I2C codec.
> > 
> > So, should it just be named "cht-rt5677", but the code inside will be Yoga
> > Book-specific without generalization? Or is it better to make the code as
> > generic as possible and add machine-specific things via some kind of quirks
> > selected by DMI data, for example? I doubt if we will meet another
> > working Cherry Trail-based device with RT5677 codec in the future.
> 
> "cht_rt5677" sounds like a good filename for this very driver. Yeah, I
> should have used '_' when mentioning <platform>_<codec> in my previous
> response, so that the naming remains cohesive with the vast majority. Oh
> well..

no problem, I am going to follow existing style anyway.

> 
> In regard to the generic/specific - I typically turn to the coverage and the
> schedule to answer such question. I believe the specific tablets you
> mentioned are the only coverage for the driver. And thus I do not see a
> reason to scale beyond that.
> 
> The generic approach is good (and usually favoured) when one provides a
> feature or a driver for specific configuration XYZ_v1 but with XYZ_v2,
> that's coming a year from now, already in mind. I do not think this is the
> case either.

So, it should be enough to rename the driver and keep all
machine-specific things as is until we need to support another
configuration variant (likely never). Sounds good for me.

> > > > +	/* register the soc card */
> > > > +	snd_soc_card_cht_yb.dev = &pdev->dev;
> > > 
> > > I'd move away from static-card approach and dynamically allocate the object
> > > here, in the probe() function.
> > 
> > hmm... OK but why? Most sound drivers use a static approach to simplify
> > declaration.
> 
> Most of the "legacy" machine-board drivers I'd say. If you take a look at
> SOF's or avs's (intel/avs/boards) the number of static cards is limited.
> 
> There is no _strong_ argument against or in favour of either. The reason I
> suggest to switch to the dynamic approach is similar to what you do here -
> base your code on someone else' work. When such copy lands on a
> configuration where not one but two I2C codecs are present, more than one
> sound card might be desired.
> With statics, things would get messy in runtime. With dynamic allocation
> developer can forget about problems related to inheriting some unwanted
> data.

Yes, avoiding of spreading a bad practice is a good reason, even for cases
where we never get more than one card instance, thanks for explanation.

> 
> Small bonus is not occupying space for the card object until the
> platform-device (that represents the sound card) is ready for probing. The
> static descriptor occupies space the moment the module is launched.

sure.


-- 
Yauhen Kharuzhy