The first patch in this series has the same commit title as the second one, can this be updated with a more meaningful description of the two patches?
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 44fccfb93cff..ba1b02ed184a 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -6721,7 +6721,7 @@ static void comp_generic_playback_hook(struct hda_pcm_stream *hinfo, struct hda_ } }
-struct cs35l41_dev_name { +struct scodec_dev_name {
you are changing things in patch_realtek.c that are completely unrelated to the tas2781, usually the recommendation is that the changes have to be part of a different patch so that the real addition of tas2781 parts are easier to review.
const char *bus; const char *hid; int index; @@ -6730,7 +6730,7 @@ struct cs35l41_dev_name { /* match the device name in a slightly relaxed manner */ static int comp_match_cs35l41_dev_name(struct device *dev, void *data) {
- struct cs35l41_dev_name *p = data;
- struct scodec_dev_name *p = data; const char *d = dev_name(dev); int n = strlen(p->bus); char tmp[32];
@@ -6746,12 +6746,32 @@ static int comp_match_cs35l41_dev_name(struct device *dev, void *data) return !strcmp(d + n, tmp); }
+static int comp_match_tas2781_dev_name(struct device *dev,
- void *data)
+{
- struct scodec_dev_name *p = data;
- const char *d = dev_name(dev);
- int n = strlen(p->bus);
- char tmp[32];
- /* check the bus name */
- if (strncmp(d, p->bus, n))
return 0;- /* skip the bus number */
- if (isdigit(d[n]))
n++;- /* the rest must be exact matching */
- snprintf(tmp, sizeof(tmp), "-%s:00", p->hid);
ACPI can sometimes add :01 suffixes, this looks like the re-invention of an ACPI helper?
Adding Andy for the ACPI review.
- return !strcmp(d + n, tmp);
+}
static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char *bus, const char *hid, int count) { struct device *dev = hda_codec_dev(cdc); struct alc_spec *spec = cdc->spec;
- struct cs35l41_dev_name *rec;
struct scodec_dev_name *rec; int ret, i;
switch (action) {
@@ -6779,6 +6799,41 @@ static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char } }
+static void tas2781_generic_fixup(struct hda_codec *cdc, int action,
- const char *bus, const char *hid)
+{
- struct device *dev = hda_codec_dev(cdc);
- struct alc_spec *spec = cdc->spec;
- struct scodec_dev_name *rec;
- int ret;
- switch (action) {
- case HDA_FIXUP_ACT_PRE_PROBE:
rec = devm_kmalloc(dev, sizeof(*rec), GFP_KERNEL);if (!rec)return;rec->bus = bus;rec->hid = hid;rec->index = 0;spec->comps[0].codec = cdc;component_match_add(dev, &spec->match,comp_match_tas2781_dev_name, rec);ret = component_master_add_with_match(dev, &comp_master_ops,spec->match);if (ret)codec_err(cdc,"Fail to register component aggregator %d\n",ret);elsespec->gen.pcm_playback_hook =comp_generic_playback_hook;break;- case HDA_FIXUP_ACT_FREE:
This is the first use of FIXUP_ACT_FREE in this function, is this required/intentional?
component_master_del(dev, &comp_master_ops);
Also is there a need to test that the PRE_PROBE actually worked?
break;- }
+}
static void cs35l41_fixup_i2c_two(struct hda_codec *cdc, const struct hda_fixup *fix, int action) { cs35l41_generic_fixup(cdc, action, "i2c", "CSC3551", 2); @@ -6806,6 +6861,12 @@ static void alc287_fixup_legion_16ithg6_speakers(struct hda_codec *cdc, const st cs35l41_generic_fixup(cdc, action, "i2c", "CLSA0101", 2); }
+static void tas2781_fixup_i2c(struct hda_codec *cdc,
- const struct hda_fixup *fix, int action)
+{
tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");
TI ACPI ID is TXNW
https://uefi.org/ACPI_ID_List?search=TEXAS
There's also a PNP ID PXN
https://uefi.org/PNP_ID_List?search=TEXAS
"TIAS" looks like an invented identifier. It's not uncommon but should be recorded with a comment if I am not mistaken.
+}
/* for alc295_fixup_hp_top_speakers */ #include "hp_x360_helper.c"
@@ -7231,6 +7292,7 @@ enum { ALC295_FIXUP_DELL_INSPIRON_TOP_SPEAKERS, ALC236_FIXUP_DELL_DUAL_CODECS, ALC287_FIXUP_CS35L41_I2C_2_THINKPAD_ACPI,
- ALC287_FIXUP_TAS2781_I2C,
};
/* A special fixup for Lenovo C940 and Yoga Duet 7; @@ -9309,6 +9371,12 @@ static const struct hda_fixup alc269_fixups[] = { .chained = true, .chain_id = ALC269_FIXUP_THINKPAD_ACPI, },
- [ALC287_FIXUP_TAS2781_I2C] = {
.type = HDA_FIXUP_FUNC,.v.func = tas2781_fixup_i2c,.chained = true,.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
If this is part of the THINKPAD chain, should this fixup name also refer to THINKPAD, as e.g. ALC287_FIXUP_CS35L41_I2C_2_THINKPAD_ACPI