2021-10-08 11:21:16

by Lucas Tanure

[permalink] [raw]
Subject: [RFC] sound: cs35l41: Add support for Legion 7 16ACHg6 laptop

Hi,

I would like to get some guidance about this solution to
support the 16ACHg6 laptop.

Hardware:
- The 16ACHg6 laptop has two CS35L41 amplifiers, connected
to Realtek ALC287 by an I2S bus and by and direct I2C to the CPU.
- The ALC287 codec is connected to the CPU by an HDA bus.
- The CS35L41 has a DSP which will require firmware to be loaded.

Architecture:
- To load the firmware for CS35L41, this solution will require
the wm_adsp library, which requires regmap, header definitions and
register tables.
- To minimize the duplication of the code, the HDA functions will
be placed inside the ASoC CS35L41 driver.
- Finally, HDA patch_realtek will access exposed functions from
ASoC CS35L41 driver to initialize the amplifiers, start and stop
streams and load firmware.

Notes:
- This is a work in progress, so the code is not functional, its
only intent is to demonstrate the overall solution
- If accepted, this will be split into a couple of patches for
a new patch chain

Signed-off-by: Lucas Tanure <[email protected]>
---
drivers/acpi/scan.c | 1 +
drivers/platform/x86/i2c-multi-instantiate.c | 7 ++
include/sound/cs35l41.h | 4 ++
sound/pci/hda/Kconfig | 1 +
sound/pci/hda/patch_realtek.c | 21 +++++-
sound/soc/codecs/cs35l41.c | 75 ++++++++++++++++----
6 files changed, 95 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 5b54c80b9d32..c1c27a408420 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1703,6 +1703,7 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
{"BSG2150", },
{"INT33FE", },
{"INT3515", },
+ {"CLSA0100", },
{}
};

diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c
index a50153ecd560..b61f7e30d42a 100644
--- a/drivers/platform/x86/i2c-multi-instantiate.c
+++ b/drivers/platform/x86/i2c-multi-instantiate.c
@@ -139,6 +139,12 @@ static const struct i2c_inst_data bsg2150_data[] = {
{}
};

+static const struct i2c_inst_data clsa0100_data[] = {
+ { "cs35l41", IRQ_RESOURCE_GPIO, 0 },
+ { "cs35l41", IRQ_RESOURCE_GPIO, 0 },
+ {}
+};
+
/*
* Device with _HID INT3515 (TI PD controllers) has some unresolved interrupt
* issues. The most common problem seen is interrupt flood.
@@ -170,6 +176,7 @@ static const struct i2c_inst_data bsg2150_data[] = {
static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = {
{ "BSG1160", (unsigned long)bsg1160_data },
{ "BSG2150", (unsigned long)bsg2150_data },
+ { "CLSA0100", (unsigned long)clsa0100_data },
{ }
};
MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids);
diff --git a/include/sound/cs35l41.h b/include/sound/cs35l41.h
index 1f1e3c6c9be1..4d665b7dbfdf 100644
--- a/include/sound/cs35l41.h
+++ b/include/sound/cs35l41.h
@@ -23,6 +23,8 @@ struct cs35l41_irq_cfg {
};

struct cs35l41_platform_data {
+ bool no_bst;
+ bool hda;
int bst_ind;
int bst_ipk;
int bst_cap;
@@ -31,4 +33,6 @@ struct cs35l41_platform_data {
struct cs35l41_irq_cfg irq_config2;
};

+void cs35l41_hda_init(void);
+
#endif /* __CS35L41_H */
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index ab9d2746e804..37202466f033 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -95,6 +95,7 @@ config SND_HDA_CODEC_REALTEK
tristate "Build Realtek HD-audio codec support"
select SND_HDA_GENERIC
select SND_HDA_GENERIC_LEDS
+ select SND_SOC_CS35L41_I2C
help
Say Y or M here to include Realtek HD-audio codec support in
snd-hda-intel driver, such as ALC880.
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 4407f7da57c4..2a0ac9a1b613 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -21,6 +21,7 @@
#include <sound/core.h>
#include <sound/jack.h>
#include <sound/hda_codec.h>
+#include <sound/cs35l41.h>
#include "hda_local.h"
#include "hda_auto_parser.h"
#include "hda_jack.h"
@@ -6443,6 +6444,18 @@ static void alc287_fixup_legion_15imhg05_speakers(struct hda_codec *codec,
}
}

+static void alc287_fixup_lenovo_y760(struct hda_codec *cdc, const struct hda_fixup *fix, int action)
+{
+ if (action == HDA_FIXUP_ACT_PROBE) {
+ codec_info(cdc, "HDA_FIXUP_ACT_PROBE\n");
+ cs35l41_hda_init();
+ } else if (action == HDA_FIXUP_ACT_INIT) {
+ codec_info(cdc, "HDA_FIXUP_ACT_INIT\n");
+ } else if (action == HDA_FIXUP_ACT_FREE) {
+ codec_info(cdc, "HDA_FIXUP_ACT_FREE\n");
+ }
+}
+
/* for alc295_fixup_hp_top_speakers */
#include "hp_x360_helper.c"

@@ -6663,7 +6676,8 @@ enum {
ALC287_FIXUP_LEGION_15IMHG05_SPEAKERS,
ALC287_FIXUP_LEGION_15IMHG05_AUTOMUTE,
ALC287_FIXUP_YOGA7_14ITL_SPEAKERS,
- ALC287_FIXUP_13S_GEN2_SPEAKERS
+ ALC287_FIXUP_13S_GEN2_SPEAKERS,
+ ALC287_FIXUP_LENOVO_Y760
};

static const struct hda_fixup alc269_fixups[] = {
@@ -8361,6 +8375,10 @@ static const struct hda_fixup alc269_fixups[] = {
.chained = true,
.chain_id = ALC269_FIXUP_HEADSET_MODE,
},
+ [ALC287_FIXUP_LENOVO_Y760] = {
+ .type = HDA_FIXUP_FUNC,
+ .v.func = alc287_fixup_lenovo_y760,
+ },
};

static const struct snd_pci_quirk alc269_fixup_tbl[] = {
@@ -8755,6 +8773,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
SND_PCI_QUIRK(0x17aa, 0x3818, "Lenovo C940", ALC298_FIXUP_LENOVO_SPK_VOLUME),
SND_PCI_QUIRK(0x17aa, 0x3827, "Ideapad S740", ALC285_FIXUP_IDEAPAD_S740_COEF),
SND_PCI_QUIRK(0x17aa, 0x3843, "Yoga 9i", ALC287_FIXUP_IDEAPAD_BASS_SPK_AMP),
+ SND_PCI_QUIRK(0x17aa, 0x3847, "Legion Y760", ALC287_FIXUP_LENOVO_Y760),
SND_PCI_QUIRK(0x17aa, 0x3813, "Legion 7i 15IMHG05", ALC287_FIXUP_LEGION_15IMHG05_SPEAKERS),
SND_PCI_QUIRK(0x17aa, 0x3852, "Lenovo Yoga 7 14ITL5", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
SND_PCI_QUIRK(0x17aa, 0x3853, "Lenovo Yoga 7 15ITL5", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
diff --git a/sound/soc/codecs/cs35l41.c b/sound/soc/codecs/cs35l41.c
index b16eb6610c0e..f643ed1b48c0 100644
--- a/sound/soc/codecs/cs35l41.c
+++ b/sound/soc/codecs/cs35l41.c
@@ -21,9 +21,17 @@
#include <sound/soc.h>
#include <sound/soc-dapm.h>
#include <sound/tlv.h>
+#include <linux/acpi.h>

#include "cs35l41.h"

+static struct list_head *cs35l41_hda_lst;
+
+struct cs35l41_hda_node {
+ struct list_head node;
+ struct cs35l41_private *cs35l41;
+};
+
static const char * const cs35l41_supplies[CS35L41_NUM_SUPPLIES] = {
"VA",
"VP",
@@ -1039,9 +1047,7 @@ static int cs35l41_set_pdata(struct cs35l41_private *cs35l41)
{
int ret;

- /* Set Platform Data */
- /* Required */
- if (cs35l41->pdata.bst_ipk &&
+ if (!cs35l41->pdata.no_bst && cs35l41->pdata.bst_ipk &&
cs35l41->pdata.bst_ind && cs35l41->pdata.bst_cap) {
ret = cs35l41_boost_config(cs35l41, cs35l41->pdata.bst_ind,
cs35l41->pdata.bst_cap,
@@ -1051,8 +1057,7 @@ static int cs35l41_set_pdata(struct cs35l41_private *cs35l41)
return ret;
}
} else {
- dev_err(cs35l41->dev, "Incomplete Boost component DT config\n");
- return -EINVAL;
+ dev_info(cs35l41->dev, "Boost disabled\n");
}

/* Optional */
@@ -1148,9 +1153,31 @@ static int cs35l41_handle_pdata(struct device *dev,
{
struct cs35l41_irq_cfg *irq_gpio1_config = &pdata->irq_config1;
struct cs35l41_irq_cfg *irq_gpio2_config = &pdata->irq_config2;
+ struct acpi_device *adev;
+ struct device *phys_dev;
unsigned int val;
int ret;

+ if (memcmp(dev_name(cs35l41->dev), "i2c-CLSA0100", 12) == 0) {
+ pdata->no_bst = true;
+ pdata->hda = true;
+ adev = acpi_dev_get_first_match_dev("CLSA0100", "1", -1);
+ if (!adev) {
+ dev_err(dev, "Failed to find an ACPI device\n");
+ return -ENODEV;
+ }
+
+ phys_dev = get_device(acpi_get_first_physical_node(adev));
+ acpi_dev_put(adev);
+
+ if (!phys_dev) {
+ dev_err(dev, "Failed to find a physical device\n");
+ return -ENODEV;
+ }
+ cs35l41->reset_gpio = gpiod_get_index(phys_dev, NULL, 0, GPIOD_ASIS);
+ return 0;
+ }
+
ret = device_property_read_u32(dev, "cirrus,boost-peak-milliamp", &val);
if (ret >= 0)
pdata->bst_ipk = val;
@@ -1237,10 +1264,22 @@ static const struct reg_sequence cs35l41_revb2_errata_patch[] = {
{ 0x00000040, 0x00003333 },
};

+void cs35l41_hda_init(void)
+{
+ struct list_head *p;
+ int i = 0;
+
+ list_for_each(p, cs35l41_hda_lst) {
+ pr_info("%s %d\n", __func__, i++);
+ }
+}
+EXPORT_SYMBOL_GPL(cs35l41_hda_init);
+
int cs35l41_probe(struct cs35l41_private *cs35l41,
struct cs35l41_platform_data *pdata)
{
u32 regid, reg_revid, i, mtl_revid, int_status, chipid_match;
+ struct cs35l41_hda_node *cs35l41_hda;
int irq_pol = 0;
int ret;

@@ -1269,8 +1308,8 @@ int cs35l41_probe(struct cs35l41_private *cs35l41,
}

/* returning NULL can be an option if in stereo mode */
- cs35l41->reset_gpio = devm_gpiod_get_optional(cs35l41->dev, "reset",
- GPIOD_OUT_LOW);
+ if (!cs35l41->reset_gpio)
+ cs35l41->reset_gpio = devm_gpiod_get_optional(cs35l41->dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(cs35l41->reset_gpio)) {
ret = PTR_ERR(cs35l41->reset_gpio);
cs35l41->reset_gpio = NULL;
@@ -1413,12 +1452,22 @@ int cs35l41_probe(struct cs35l41_private *cs35l41,
goto err;
}

- ret = devm_snd_soc_register_component(cs35l41->dev,
- &soc_component_dev_cs35l41,
- cs35l41_dai, ARRAY_SIZE(cs35l41_dai));
- if (ret < 0) {
- dev_err(cs35l41->dev, "Register codec failed: %d\n", ret);
- goto err;
+ if (!cs35l41->pdata.hda) {
+ ret = devm_snd_soc_register_component(cs35l41->dev,
+ &soc_component_dev_cs35l41,
+ cs35l41_dai, ARRAY_SIZE(cs35l41_dai));
+ if (ret < 0) {
+ dev_err(cs35l41->dev, "Register codec failed: %d\n", ret);
+ goto err;
+ }
+ } else {
+ if (!cs35l41_hda_lst) {
+ cs35l41_hda_lst = devm_kzalloc(cs35l41->dev, sizeof(*cs35l41_hda_lst),
+ GFP_KERNEL);
+ INIT_LIST_HEAD(cs35l41_hda_lst);
+ }
+ cs35l41_hda = devm_kzalloc(cs35l41->dev, sizeof(*cs35l41_hda), GFP_KERNEL);
+ list_add(&cs35l41_hda->node, cs35l41_hda_lst);
}

dev_info(cs35l41->dev, "Cirrus Logic CS35L41 (%x), Revision: %02X\n",
--
2.33.0


2021-10-12 20:23:55

by Takashi Iwai

[permalink] [raw]
Subject: Re: [RFC] sound: cs35l41: Add support for Legion 7 16ACHg6 laptop

On Fri, 08 Oct 2021 13:19:02 +0200,
Lucas Tanure wrote:
>
> Hi,
>
> I would like to get some guidance about this solution to
> support the 16ACHg6 laptop.
>
> Hardware:
> - The 16ACHg6 laptop has two CS35L41 amplifiers, connected
> to Realtek ALC287 by an I2S bus and by and direct I2C to the CPU.
> - The ALC287 codec is connected to the CPU by an HDA bus.
> - The CS35L41 has a DSP which will require firmware to be loaded.
>
> Architecture:
> - To load the firmware for CS35L41, this solution will require
> the wm_adsp library, which requires regmap, header definitions and
> register tables.
> - To minimize the duplication of the code, the HDA functions will
> be placed inside the ASoC CS35L41 driver.
> - Finally, HDA patch_realtek will access exposed functions from
> ASoC CS35L41 driver to initialize the amplifiers, start and stop
> streams and load firmware.

Through a very quick glance, a potential problem is that this design
would make the HD-audio codec driver dependent on those other ASoC
ones. As the Realtek HD-audio codec driver is used by quite many
other people, we'd like to reduce such dependency mess.

Maybe a dynamic binding with component framework can be used?

Alternatively, we may build up a stuff on top of ASoC like what SOF
driver did. It'll be another lot of work, though.


thanks,

Takashi

> Notes:
> - This is a work in progress, so the code is not functional, its
> only intent is to demonstrate the overall solution
> - If accepted, this will be split into a couple of patches for
> a new patch chain
>
> Signed-off-by: Lucas Tanure <[email protected]>
> ---
> drivers/acpi/scan.c | 1 +
> drivers/platform/x86/i2c-multi-instantiate.c | 7 ++
> include/sound/cs35l41.h | 4 ++
> sound/pci/hda/Kconfig | 1 +
> sound/pci/hda/patch_realtek.c | 21 +++++-
> sound/soc/codecs/cs35l41.c | 75 ++++++++++++++++----
> 6 files changed, 95 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 5b54c80b9d32..c1c27a408420 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1703,6 +1703,7 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
> {"BSG2150", },
> {"INT33FE", },
> {"INT3515", },
> + {"CLSA0100", },
> {}
> };
>
> diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c
> index a50153ecd560..b61f7e30d42a 100644
> --- a/drivers/platform/x86/i2c-multi-instantiate.c
> +++ b/drivers/platform/x86/i2c-multi-instantiate.c
> @@ -139,6 +139,12 @@ static const struct i2c_inst_data bsg2150_data[] = {
> {}
> };
>
> +static const struct i2c_inst_data clsa0100_data[] = {
> + { "cs35l41", IRQ_RESOURCE_GPIO, 0 },
> + { "cs35l41", IRQ_RESOURCE_GPIO, 0 },
> + {}
> +};
> +
> /*
> * Device with _HID INT3515 (TI PD controllers) has some unresolved interrupt
> * issues. The most common problem seen is interrupt flood.
> @@ -170,6 +176,7 @@ static const struct i2c_inst_data bsg2150_data[] = {
> static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = {
> { "BSG1160", (unsigned long)bsg1160_data },
> { "BSG2150", (unsigned long)bsg2150_data },
> + { "CLSA0100", (unsigned long)clsa0100_data },
> { }
> };
> MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids);
> diff --git a/include/sound/cs35l41.h b/include/sound/cs35l41.h
> index 1f1e3c6c9be1..4d665b7dbfdf 100644
> --- a/include/sound/cs35l41.h
> +++ b/include/sound/cs35l41.h
> @@ -23,6 +23,8 @@ struct cs35l41_irq_cfg {
> };
>
> struct cs35l41_platform_data {
> + bool no_bst;
> + bool hda;
> int bst_ind;
> int bst_ipk;
> int bst_cap;
> @@ -31,4 +33,6 @@ struct cs35l41_platform_data {
> struct cs35l41_irq_cfg irq_config2;
> };
>
> +void cs35l41_hda_init(void);
> +
> #endif /* __CS35L41_H */
> diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
> index ab9d2746e804..37202466f033 100644
> --- a/sound/pci/hda/Kconfig
> +++ b/sound/pci/hda/Kconfig
> @@ -95,6 +95,7 @@ config SND_HDA_CODEC_REALTEK
> tristate "Build Realtek HD-audio codec support"
> select SND_HDA_GENERIC
> select SND_HDA_GENERIC_LEDS
> + select SND_SOC_CS35L41_I2C
> help
> Say Y or M here to include Realtek HD-audio codec support in
> snd-hda-intel driver, such as ALC880.
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 4407f7da57c4..2a0ac9a1b613 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -21,6 +21,7 @@
> #include <sound/core.h>
> #include <sound/jack.h>
> #include <sound/hda_codec.h>
> +#include <sound/cs35l41.h>
> #include "hda_local.h"
> #include "hda_auto_parser.h"
> #include "hda_jack.h"
> @@ -6443,6 +6444,18 @@ static void alc287_fixup_legion_15imhg05_speakers(struct hda_codec *codec,
> }
> }
>
> +static void alc287_fixup_lenovo_y760(struct hda_codec *cdc, const struct hda_fixup *fix, int action)
> +{
> + if (action == HDA_FIXUP_ACT_PROBE) {
> + codec_info(cdc, "HDA_FIXUP_ACT_PROBE\n");
> + cs35l41_hda_init();
> + } else if (action == HDA_FIXUP_ACT_INIT) {
> + codec_info(cdc, "HDA_FIXUP_ACT_INIT\n");
> + } else if (action == HDA_FIXUP_ACT_FREE) {
> + codec_info(cdc, "HDA_FIXUP_ACT_FREE\n");
> + }
> +}
> +
> /* for alc295_fixup_hp_top_speakers */
> #include "hp_x360_helper.c"
>
> @@ -6663,7 +6676,8 @@ enum {
> ALC287_FIXUP_LEGION_15IMHG05_SPEAKERS,
> ALC287_FIXUP_LEGION_15IMHG05_AUTOMUTE,
> ALC287_FIXUP_YOGA7_14ITL_SPEAKERS,
> - ALC287_FIXUP_13S_GEN2_SPEAKERS
> + ALC287_FIXUP_13S_GEN2_SPEAKERS,
> + ALC287_FIXUP_LENOVO_Y760
> };
>
> static const struct hda_fixup alc269_fixups[] = {
> @@ -8361,6 +8375,10 @@ static const struct hda_fixup alc269_fixups[] = {
> .chained = true,
> .chain_id = ALC269_FIXUP_HEADSET_MODE,
> },
> + [ALC287_FIXUP_LENOVO_Y760] = {
> + .type = HDA_FIXUP_FUNC,
> + .v.func = alc287_fixup_lenovo_y760,
> + },
> };
>
> static const struct snd_pci_quirk alc269_fixup_tbl[] = {
> @@ -8755,6 +8773,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
> SND_PCI_QUIRK(0x17aa, 0x3818, "Lenovo C940", ALC298_FIXUP_LENOVO_SPK_VOLUME),
> SND_PCI_QUIRK(0x17aa, 0x3827, "Ideapad S740", ALC285_FIXUP_IDEAPAD_S740_COEF),
> SND_PCI_QUIRK(0x17aa, 0x3843, "Yoga 9i", ALC287_FIXUP_IDEAPAD_BASS_SPK_AMP),
> + SND_PCI_QUIRK(0x17aa, 0x3847, "Legion Y760", ALC287_FIXUP_LENOVO_Y760),
> SND_PCI_QUIRK(0x17aa, 0x3813, "Legion 7i 15IMHG05", ALC287_FIXUP_LEGION_15IMHG05_SPEAKERS),
> SND_PCI_QUIRK(0x17aa, 0x3852, "Lenovo Yoga 7 14ITL5", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
> SND_PCI_QUIRK(0x17aa, 0x3853, "Lenovo Yoga 7 15ITL5", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
> diff --git a/sound/soc/codecs/cs35l41.c b/sound/soc/codecs/cs35l41.c
> index b16eb6610c0e..f643ed1b48c0 100644
> --- a/sound/soc/codecs/cs35l41.c
> +++ b/sound/soc/codecs/cs35l41.c
> @@ -21,9 +21,17 @@
> #include <sound/soc.h>
> #include <sound/soc-dapm.h>
> #include <sound/tlv.h>
> +#include <linux/acpi.h>
>
> #include "cs35l41.h"
>
> +static struct list_head *cs35l41_hda_lst;
> +
> +struct cs35l41_hda_node {
> + struct list_head node;
> + struct cs35l41_private *cs35l41;
> +};
> +
> static const char * const cs35l41_supplies[CS35L41_NUM_SUPPLIES] = {
> "VA",
> "VP",
> @@ -1039,9 +1047,7 @@ static int cs35l41_set_pdata(struct cs35l41_private *cs35l41)
> {
> int ret;
>
> - /* Set Platform Data */
> - /* Required */
> - if (cs35l41->pdata.bst_ipk &&
> + if (!cs35l41->pdata.no_bst && cs35l41->pdata.bst_ipk &&
> cs35l41->pdata.bst_ind && cs35l41->pdata.bst_cap) {
> ret = cs35l41_boost_config(cs35l41, cs35l41->pdata.bst_ind,
> cs35l41->pdata.bst_cap,
> @@ -1051,8 +1057,7 @@ static int cs35l41_set_pdata(struct cs35l41_private *cs35l41)
> return ret;
> }
> } else {
> - dev_err(cs35l41->dev, "Incomplete Boost component DT config\n");
> - return -EINVAL;
> + dev_info(cs35l41->dev, "Boost disabled\n");
> }
>
> /* Optional */
> @@ -1148,9 +1153,31 @@ static int cs35l41_handle_pdata(struct device *dev,
> {
> struct cs35l41_irq_cfg *irq_gpio1_config = &pdata->irq_config1;
> struct cs35l41_irq_cfg *irq_gpio2_config = &pdata->irq_config2;
> + struct acpi_device *adev;
> + struct device *phys_dev;
> unsigned int val;
> int ret;
>
> + if (memcmp(dev_name(cs35l41->dev), "i2c-CLSA0100", 12) == 0) {
> + pdata->no_bst = true;
> + pdata->hda = true;
> + adev = acpi_dev_get_first_match_dev("CLSA0100", "1", -1);
> + if (!adev) {
> + dev_err(dev, "Failed to find an ACPI device\n");
> + return -ENODEV;
> + }
> +
> + phys_dev = get_device(acpi_get_first_physical_node(adev));
> + acpi_dev_put(adev);
> +
> + if (!phys_dev) {
> + dev_err(dev, "Failed to find a physical device\n");
> + return -ENODEV;
> + }
> + cs35l41->reset_gpio = gpiod_get_index(phys_dev, NULL, 0, GPIOD_ASIS);
> + return 0;
> + }
> +
> ret = device_property_read_u32(dev, "cirrus,boost-peak-milliamp", &val);
> if (ret >= 0)
> pdata->bst_ipk = val;
> @@ -1237,10 +1264,22 @@ static const struct reg_sequence cs35l41_revb2_errata_patch[] = {
> { 0x00000040, 0x00003333 },
> };
>
> +void cs35l41_hda_init(void)
> +{
> + struct list_head *p;
> + int i = 0;
> +
> + list_for_each(p, cs35l41_hda_lst) {
> + pr_info("%s %d\n", __func__, i++);
> + }
> +}
> +EXPORT_SYMBOL_GPL(cs35l41_hda_init);
> +
> int cs35l41_probe(struct cs35l41_private *cs35l41,
> struct cs35l41_platform_data *pdata)
> {
> u32 regid, reg_revid, i, mtl_revid, int_status, chipid_match;
> + struct cs35l41_hda_node *cs35l41_hda;
> int irq_pol = 0;
> int ret;
>
> @@ -1269,8 +1308,8 @@ int cs35l41_probe(struct cs35l41_private *cs35l41,
> }
>
> /* returning NULL can be an option if in stereo mode */
> - cs35l41->reset_gpio = devm_gpiod_get_optional(cs35l41->dev, "reset",
> - GPIOD_OUT_LOW);
> + if (!cs35l41->reset_gpio)
> + cs35l41->reset_gpio = devm_gpiod_get_optional(cs35l41->dev, "reset", GPIOD_OUT_LOW);
> if (IS_ERR(cs35l41->reset_gpio)) {
> ret = PTR_ERR(cs35l41->reset_gpio);
> cs35l41->reset_gpio = NULL;
> @@ -1413,12 +1452,22 @@ int cs35l41_probe(struct cs35l41_private *cs35l41,
> goto err;
> }
>
> - ret = devm_snd_soc_register_component(cs35l41->dev,
> - &soc_component_dev_cs35l41,
> - cs35l41_dai, ARRAY_SIZE(cs35l41_dai));
> - if (ret < 0) {
> - dev_err(cs35l41->dev, "Register codec failed: %d\n", ret);
> - goto err;
> + if (!cs35l41->pdata.hda) {
> + ret = devm_snd_soc_register_component(cs35l41->dev,
> + &soc_component_dev_cs35l41,
> + cs35l41_dai, ARRAY_SIZE(cs35l41_dai));
> + if (ret < 0) {
> + dev_err(cs35l41->dev, "Register codec failed: %d\n", ret);
> + goto err;
> + }
> + } else {
> + if (!cs35l41_hda_lst) {
> + cs35l41_hda_lst = devm_kzalloc(cs35l41->dev, sizeof(*cs35l41_hda_lst),
> + GFP_KERNEL);
> + INIT_LIST_HEAD(cs35l41_hda_lst);
> + }
> + cs35l41_hda = devm_kzalloc(cs35l41->dev, sizeof(*cs35l41_hda), GFP_KERNEL);
> + list_add(&cs35l41_hda->node, cs35l41_hda_lst);
> }
>
> dev_info(cs35l41->dev, "Cirrus Logic CS35L41 (%x), Revision: %02X\n",
> --
> 2.33.0
>

2021-10-13 16:18:49

by Takashi Iwai

[permalink] [raw]
Subject: Re: [RFC] sound: cs35l41: Add support for Legion 7 16ACHg6 laptop

On Tue, 12 Oct 2021 22:22:07 +0200,
Takashi Iwai wrote:
>
> On Fri, 08 Oct 2021 13:19:02 +0200,
> Lucas Tanure wrote:
> >
> > Hi,
> >
> > I would like to get some guidance about this solution to
> > support the 16ACHg6 laptop.
> >
> > Hardware:
> > - The 16ACHg6 laptop has two CS35L41 amplifiers, connected
> > to Realtek ALC287 by an I2S bus and by and direct I2C to the CPU.
> > - The ALC287 codec is connected to the CPU by an HDA bus.
> > - The CS35L41 has a DSP which will require firmware to be loaded.
> >
> > Architecture:
> > - To load the firmware for CS35L41, this solution will require
> > the wm_adsp library, which requires regmap, header definitions and
> > register tables.
> > - To minimize the duplication of the code, the HDA functions will
> > be placed inside the ASoC CS35L41 driver.
> > - Finally, HDA patch_realtek will access exposed functions from
> > ASoC CS35L41 driver to initialize the amplifiers, start and stop
> > streams and load firmware.
>
> Through a very quick glance, a potential problem is that this design
> would make the HD-audio codec driver dependent on those other ASoC
> ones. As the Realtek HD-audio codec driver is used by quite many
> other people, we'd like to reduce such dependency mess.
>
> Maybe a dynamic binding with component framework can be used?
>
> Alternatively, we may build up a stuff on top of ASoC like what SOF
> driver did. It'll be another lot of work, though.

Or, yet another (and easier) alternative would be to create a new
codec driver that is specific to vendor+subsystem pair. We'll need to
extend the hda_device_id and its matching mechanism, and the realtek
codec driver needs to exclude the matching with the given SSID
explicitly.

A patch below is an example.


Takashi

---
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index ae2e75d15b21..5558f2ba2fcf 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -248,6 +248,7 @@ struct serio_device_id {

struct hda_device_id {
__u32 vendor_id;
+ __u32 subsystem_id;
__u32 rev_id;
__u8 api_version;
const char *name;
diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index 0e45963bb767..3e316a798361 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -79,10 +79,12 @@ typedef int (*hda_codec_patch_t)(struct hda_codec *);
#define HDA_CODEC_ID_GENERIC_HDMI 0x00000101
#define HDA_CODEC_ID_GENERIC 0x00000201

-#define HDA_CODEC_REV_ENTRY(_vid, _rev, _name, _patch) \
- { .vendor_id = (_vid), .rev_id = (_rev), .name = (_name), \
- .api_version = HDA_DEV_LEGACY, \
+#define HDA_CODEC_FULL_ENTRY(_vid, _subsystem, _rev, _name, _patch) \
+ { .vendor_id = (_vid), .subsystem_id = (_subsystem), .rev_id = (_rev), \
+ .name = (_name), .api_version = HDA_DEV_LEGACY, \
.driver_data = (unsigned long)(_patch) }
+#define HDA_CODEC_REV_ENTRY(_vid, _rev, _name, _patch) \
+ HDA_CODEC_FULL_ENTRY(_vid, 0, _rev, _name, _patch)
#define HDA_CODEC_ENTRY(_vid, _name, _patch) \
HDA_CODEC_REV_ENTRY(_vid, 0, _name, _patch)

diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index cc3625617a0e..641b4f9bb2be 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -211,6 +211,7 @@ int main(void)

DEVID(hda_device_id);
DEVID_FIELD(hda_device_id, vendor_id);
+ DEVID_FIELD(hda_device_id, subsystem_id);
DEVID_FIELD(hda_device_id, rev_id);
DEVID_FIELD(hda_device_id, api_version);

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 49aba862073e..d8faf0065c95 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1255,15 +1255,17 @@ static int do_ulpi_entry(const char *filename, void *symval,
return 1;
}

-/* Looks like: hdaudio:vNrNaN */
+/* Looks like: hdaudio:vNsNrNaN */
static int do_hda_entry(const char *filename, void *symval, char *alias)
{
DEF_FIELD(symval, hda_device_id, vendor_id);
+ DEF_FIELD(symval, hda_device_id, subsystem_id);
DEF_FIELD(symval, hda_device_id, rev_id);
DEF_FIELD(symval, hda_device_id, api_version);

strcpy(alias, "hdaudio:");
ADD(alias, "v", vendor_id != 0, vendor_id);
+ ADD(alias, "s", subsystem_id != 0, subsystem_id);
ADD(alias, "r", rev_id != 0, rev_id);
ADD(alias, "a", api_version != 0, api_version);

diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
index 3e9e9ac804f6..662abd40ca6a 100644
--- a/sound/hda/hdac_device.c
+++ b/sound/hda/hdac_device.c
@@ -206,8 +206,9 @@ EXPORT_SYMBOL_GPL(snd_hdac_device_set_chip_name);
*/
int snd_hdac_codec_modalias(struct hdac_device *codec, char *buf, size_t size)
{
- return scnprintf(buf, size, "hdaudio:v%08Xr%08Xa%02X\n",
- codec->vendor_id, codec->revision_id, codec->type);
+ return scnprintf(buf, size, "hdaudio:v%08Xs%08Xr%08Xa%02X\n",
+ codec->vendor_id, codec->subsystem_id,
+ codec->revision_id, codec->type);
}
EXPORT_SYMBOL_GPL(snd_hdac_codec_modalias);

diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile
index b8fa682ce66a..9f559773bf99 100644
--- a/sound/pci/hda/Makefile
+++ b/sound/pci/hda/Makefile
@@ -15,6 +15,7 @@ CFLAGS_hda_intel.o := -I$(src)

snd-hda-codec-generic-objs := hda_generic.o
snd-hda-codec-realtek-objs := patch_realtek.o
+snd-hda-codec-test-objs := patch_test.o
snd-hda-codec-cmedia-objs := patch_cmedia.o
snd-hda-codec-analog-objs := patch_analog.o
snd-hda-codec-idt-objs := patch_sigmatel.o
@@ -32,7 +33,7 @@ obj-$(CONFIG_SND_HDA) := snd-hda-codec.o

# codec drivers
obj-$(CONFIG_SND_HDA_GENERIC) += snd-hda-codec-generic.o
-obj-$(CONFIG_SND_HDA_CODEC_REALTEK) += snd-hda-codec-realtek.o
+obj-$(CONFIG_SND_HDA_CODEC_REALTEK) += snd-hda-codec-realtek.o snd-hda-codec-test.o
obj-$(CONFIG_SND_HDA_CODEC_CMEDIA) += snd-hda-codec-cmedia.o
obj-$(CONFIG_SND_HDA_CODEC_ANALOG) += snd-hda-codec-analog.o
obj-$(CONFIG_SND_HDA_CODEC_SIGMATEL) += snd-hda-codec-idt.o
diff --git a/sound/pci/hda/hda_bind.c b/sound/pci/hda/hda_bind.c
index 1c8bffc3eec6..367f220ec91e 100644
--- a/sound/pci/hda/hda_bind.c
+++ b/sound/pci/hda/hda_bind.c
@@ -200,7 +200,7 @@ static inline bool codec_probed(struct hda_codec *codec)
static void request_codec_module(struct hda_codec *codec)
{
#ifdef MODULE
- char modalias[32];
+ char modalias[64];
const char *mod = NULL;

switch (codec->probe_id) {
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 22d27b12c4e7..993b49554457 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -16,6 +16,7 @@
#include <linux/pci.h>
#include <linux/dmi.h>
#include <linux/module.h>
+#include <linux/export.h>
#include <linux/input.h>
#include <linux/leds.h>
#include <sound/core.h>
@@ -9510,7 +9511,7 @@ static void alc269_fill_coef(struct hda_codec *codec)

/*
*/
-static int patch_alc269(struct hda_codec *codec)
+int snd_hda_codec_realtek_alc269_probe(struct hda_codec *codec)
{
struct alc_spec *spec;
int err;
@@ -9667,6 +9668,9 @@ static int patch_alc269(struct hda_codec *codec)

alc_pre_init(codec);

+ if (codec->fixup_id != HDA_FIXUP_ID_NOT_SET)
+ goto skip_pick_fixup;
+
snd_hda_pick_fixup(codec, alc269_fixup_models,
alc269_fixup_tbl, alc269_fixups);
/* FIXME: both TX300 and ROG Strix G17 have the same SSID, and
@@ -9683,6 +9687,8 @@ static int patch_alc269(struct hda_codec *codec)
snd_hda_pick_pin_fixup(codec, alc269_fallback_pin_fixup_tbl, alc269_fixups, false);
snd_hda_pick_fixup(codec, NULL, alc269_fixup_vendor_tbl,
alc269_fixups);
+
+ skip_pick_fixup:
snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);

alc_auto_parse_customize_define(codec);
@@ -9709,6 +9715,18 @@ static int patch_alc269(struct hda_codec *codec)
alc_free(codec);
return err;
}
+EXPORT_SYMBOL(snd_hda_codec_realtek_alc269_probe);
+
+static int patch_alc269(struct hda_codec *codec)
+{
+ if (codec->core.vendor_id == 0x10ec0298 &&
+ codec->core.subsystem_id == 0x102806e5) {
+ pr_info("XXX realtek codec driver: skipping\n");
+ return -ENODEV;
+ }
+
+ return snd_hda_codec_realtek_alc269_probe(codec);
+}

/*
* ALC861
diff --git a/sound/pci/hda/patch_test.c b/sound/pci/hda/patch_test.c
new file mode 100644
index 000000000000..9070cc075af0
--- /dev/null
+++ b/sound/pci/hda/patch_test.c
@@ -0,0 +1,31 @@
+#include <linux/init.h>
+#include <linux/module.h>
+#include <sound/core.h>
+#include <sound/hda_codec.h>
+
+int snd_hda_codec_realtek_alc269_probe(struct hda_codec *codec);
+
+// static const struct hda_fixup test_fixup = { ... };
+
+static int test_probe(struct hda_codec *codec)
+{
+ pr_info("XXX forked driver\n");
+ // codec->fixup_id = 0;
+ // codec->fixup_list = &test_fixup;
+ return snd_hda_codec_realtek_alc269_probe(codec);
+}
+
+static const struct hda_device_id snd_hda_id_test[] = {
+ HDA_CODEC_FULL_ENTRY(0x10ec0298, 0x102806e5, 0, test_probe),
+ {} /* terminator */
+};
+MODULE_DEVICE_TABLE(hdaudio, snd_hda_id_test);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Test HD-audio codec");
+
+static struct hda_codec_driver test_driver = {
+ .id = snd_hda_id_test,
+};
+
+module_hda_codec_driver(test_driver);
--
2.31.1