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

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");
>

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

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.

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