Thanks Pierre. I will modify these changes. And as previously as you mentioned, I reply below to be confirmed. If they are all good as well, I will send the v3 patch. > > @@ -413,14 +488,6 @@ static struct snd_soc_codec_conf rt1011_conf[] = { > > .dlc = COMP_CODEC_CONF("i2c-10EC1011:01"), > > .name_prefix = "WR", > > }, > > - { > > - .dlc = COMP_CODEC_CONF("i2c-10EC1011:02"), > > - .name_prefix = "TL", > > - }, > > - { > > - .dlc = COMP_CODEC_CONF("i2c-10EC1011:03"), > > - .name_prefix = "TR", > > - }, > > is there a reason to remove the prefixes for Tweeters? Or is this handled > somewhere else? Yes, the "TL", "TR" name_prefix handling is moving to snd_cml_rt1011_probe() allocated by rt1011_dais_confs[] > > + for (i = 0; i < SPK_CH; i++) { > > + rt1011_dais_confs[i].dlc.name = > devm_kasprintf(&pdev->dev, > > + GFP_KERNEL, > > + "i2c- > 10EC1011:0%d", > > + i); > > if (!rt1011_dais_confs[i].dlc.name) > return -ENOMEM; > > > + switch (i) { > > + case 0: > > + rt1011_dais_confs[i].name_prefix = "WL"; > > + break; > > + > > + case 1: > > + rt1011_dais_confs[i].name_prefix = "WR"; > > + break; > > + case 2: > > + rt1011_dais_confs[i].name_prefix = "TL"; > > + break; > > + case 3: > > + rt1011_dais_confs[i].name_prefix = "TR"; > > + break; default: return -EINVAL; > > + } > > How does this work? the rt1011_dais_confs only has 2 components now? > This would probably generate a NULL access or generate an out-of-bounds > access in the array. By default there were 2 components allocated by rt1011_conf[]. However, rt1011_dais_confs has 4 components when woofers+tweeters support. BTW, I should add the default case to handle the invalid values as well. >> Subject: [PATCH v2] ASoC: Intel: boards: add stereo playback by woofer >> speaker >> support woofer stereo speakers individually >Both the commit title and message are a bit misleading. should be something like >" >ASoC: Intel: boards: cml_rt1011: split woofer and tweeter support >Support Woofer stereo speakers by default and optionally Tweeter stereo speakers with a DMI quirk " Modified the title and messages. > > @@ -302,10 +378,8 @@ SND_SOC_DAILINK_DEF(ssp1_pin, > > DAILINK_COMP_ARRAY(COMP_CPU("SSP1 Pin"))); > > SND_SOC_DAILINK_DEF(ssp1_codec, > > DAILINK_COMP_ARRAY( > > - /* WL */ COMP_CODEC("i2c-10EC1011:00", > CML_RT1011_CODEC_DAI), > > - /* WR */ COMP_CODEC("i2c-10EC1011:01", > CML_RT1011_CODEC_DAI), > > - /* TL */ COMP_CODEC("i2c-10EC1011:02", > CML_RT1011_CODEC_DAI), > > - /* TR */ COMP_CODEC("i2c-10EC1011:03", > CML_RT1011_CODEC_DAI))); > > + /* WL */ COMP_CODEC("i2c-10EC1011:00", > CML_RT1011_CODEC_DAI), > > + /* WR */ COMP_CODEC("i2c-10EC1011:01", > > + CML_RT1011_CODEC_DAI))); > > is the alignment change needed? No. Fixed. > > > @@ -456,6 +525,65 @@ static int snd_cml_rt1011_probe(struct > platform_device *pdev) > > snd_soc_card_cml.dev = &pdev->dev; > > platform_name = mach->mach_params.platform; > > > > + dmi_check_system(sof_rt1011_quirk_table); > > + > > + dev_info(&pdev->dev, "sof_rt1011_quirk = %lx\n", sof_rt1011_quirk); > > + > > + if (sof_rt1011_quirk & (SOF_RT1011_SPEAKER_TL | > > + SOF_RT1011_SPEAKER_TR)) { > > + rt1011_dais_confs = devm_kzalloc(&pdev->dev, > > + sizeof(struct snd_soc_codec_conf) * > > + SPK_CH, GFP_KERNEL); > > + > > + if (!rt1011_dais_confs) > > + return -ENOMEM; > > + > > + rt1011_dais_components = devm_kzalloc(&pdev->dev, > > + sizeof(struct > snd_soc_dai_link_component) * > > + SPK_CH, GFP_KERNEL); > > + > > + if (!rt1011_dais_components) > > + return -ENOMEM; > > + > > + for (i = 0; i < SPK_CH; i++) { > > + rt1011_dais_confs[i].dlc.name = > devm_kasprintf(&pdev->dev, > > + GFP_KERNEL, > > + "i2c- > 10EC1011:0%d", > > + i); > > if (!rt1011_dais_confs[i].dlc.name) > return -ENOMEM; added the checks. > > > + switch (i) { > > + case 0: > > + rt1011_dais_confs[i].name_prefix = "WL"; > > + break; > > + > > spurious newline? Removed the newline. > > > + case 1: