Hi Mac,
+#define SOF_RT1011_SPEAKER_WL BIT(0) +#define SOF_RT1011_SPEAKER_WR BIT(1) +#define SOF_RT1011_SPEAKER_TL BIT(2) +#define SOF_RT1011_SPEAKER_TR BIT(3) +#define SPK_CH 4
use a prefix maybe for consistency? It's also unclear why this is needed when you can have 2 or more channels, and looking below
+/* Default: Woofer+Tweeter speakers */
It's more like ALL devices have Woofers. Some devices don't have tweeters.
the WL and WR quirks are always on apparently.
+static unsigned long sof_rt1011_quirk = SOF_RT1011_SPEAKER_WL | SOF_RT1011_SPEAKER_WR |
SOF_RT1011_SPEAKER_TL | SOF_RT1011_SPEAKER_TR;+static int sof_rt1011_quirk_cb(const struct dmi_system_id *id) +{
- sof_rt1011_quirk = (unsigned long)id->driver_data;
- return 1;
+}
+static const struct dmi_system_id sof_rt1011_quirk_table[] = {
- {
.callback = sof_rt1011_quirk_cb,.matches = {DMI_MATCH(DMI_SYS_VENDOR, "Google"),DMI_MATCH(DMI_PRODUCT_NAME, "Palkia"),- },
.driver_data = (void *)(SOF_RT1011_SPEAKER_WL |SOF_RT1011_SPEAKER_WR),- },
- {
- }
+};
+static const struct snd_soc_dapm_widget cml_rt1011_tt_widgets[] = {
- SND_SOC_DAPM_SPK("TL Ext Spk", NULL),
- SND_SOC_DAPM_SPK("TR Ext Spk", NULL),
+};
- static const struct snd_soc_dapm_route cml_rt1011_rt5682_map[] = { /*speaker*/ {"TL Ext Spk", NULL, "TL SPO"},
Something's not right, if I look at the code after applying this patch I get:
static const struct snd_soc_dapm_route cml_rt1011_rt5682_map[] = { /*speaker*/ {"TL Ext Spk", NULL, "TL SPO"}, {"TR Ext Spk", NULL, "TR SPO"},
That's duplicaged from [1]
@@ -82,6 +118,12 @@ static const struct snd_soc_dapm_route cml_rt1011_rt5682_map[] = { {"DMic", NULL, "SoC DMIC"}, };
+static const struct snd_soc_dapm_route cml_rt1011_tt_map[] = {
- /*TL/TR speaker*/
- {"TL Ext Spk", NULL, "TL SPO" },
- {"TR Ext Spk", NULL, "TR SPO" },
+};
[1] we should remove the tweeeter maps in cml_rt1011_rt5682_map, no?
static int cml_rt5682_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -192,31 +263,52 @@ static int cml_rt1011_hw_params(struct snd_pcm_substream *substream, * The feedback is captured for each codec individually. * Hence all 4 codecs use 1 Tx slot each for feedback. */
if (!strcmp(codec_dai->component->name, "i2c-10EC1011:00")) {ret = snd_soc_dai_set_tdm_slot(codec_dai,0x4, 0x1, 4, 24);if (ret < 0)break;}if (!strcmp(codec_dai->component->name, "i2c-10EC1011:02")) {ret = snd_soc_dai_set_tdm_slot(codec_dai,0x1, 0x1, 4, 24);if (ret < 0)break;}/* TDM Rx slot 2 is used for Right Woofer & Tweeters pair */if (!strcmp(codec_dai->component->name, "i2c-10EC1011:01")) {ret = snd_soc_dai_set_tdm_slot(codec_dai,0x8, 0x2, 4, 24);if (ret < 0)break;}if (!strcmp(codec_dai->component->name, "i2c-10EC1011:03")) {ret = snd_soc_dai_set_tdm_slot(codec_dai,0x2, 0x2, 4, 24);if (ret < 0)break;
if (sof_rt1011_quirk & (SOF_RT1011_SPEAKER_TL |SOF_RT1011_SPEAKER_TR)) {if (!strcmp(codec_dai->component->name, "i2c-10EC1011:00")) {ret = snd_soc_dai_set_tdm_slot(codec_dai,0x4, 0x1, 4, 24);if (ret < 0)break;}if (!strcmp(codec_dai->component->name, "i2c-10EC1011:02")) {ret = snd_soc_dai_set_tdm_slot(codec_dai,0x1, 0x1, 4, 24);if (ret < 0)break;}/* TDM Rx slot 2 is used for Right Woofer & Tweeters pair */if (!strcmp(codec_dai->component->name, "i2c-10EC1011:01")) {ret = snd_soc_dai_set_tdm_slot(codec_dai,0x8, 0x2, 4, 24);if (ret < 0)break;}if (!strcmp(codec_dai->component->name, "i2c-10EC1011:03")) {ret = snd_soc_dai_set_tdm_slot(codec_dai,0x2, 0x2, 4, 24);if (ret < 0)break;}} else {if (!strcmp(codec_dai->component->name, "i2c-10EC1011:00")) {ret = snd_soc_dai_set_tdm_slot(codec_dai,0x4, 0x1, 4, 24);if (ret < 0)break;}if (!strcmp(codec_dai->component->name, "i2c-10EC1011:01")) {ret = snd_soc_dai_set_tdm_slot(codec_dai,0x8, 0x2, 4, 24);if (ret < 0)break;}
the if/else case can be simplified. The baseline is two woofers, so they can be added unconditionally, and then you can add what's missing for the tweeters. That way you have a consistent way of handling the TL/TR quirk.
static int snd_cml_rt1011_probe(struct platform_device *pdev) {
- struct snd_soc_dai_link_component *rt1011_dais_components;
- struct snd_soc_codec_conf *rt1011_dais_confs; struct card_private *ctx; struct snd_soc_acpi_mach *mach; const char *platform_name;
- int ret;
int ret, i;
ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
D'oh! Did we again let this slip in?
cml_rt1011_rt5682.c: ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC); sof_da7219_max98373.c: ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
This should be fixed in a separate patch, we don't need th ATOMIC attribute in any machine drivers - copy-paste!
if (!ctx) @@ -456,6 +541,59 @@ 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);rt1011_dais_components = devm_kzalloc(&pdev->dev,sizeof(struct snd_soc_dai_link_component) *SPK_CH, GFP_KERNEL);for (i = 0; i < SPK_CH; i++) {rt1011_dais_confs[i].dlc.name = devm_kasprintf(&pdev->dev,GFP_KERNEL,"i2c-10EC1011:0%d",i);
Check for NULL and return -ENOMEM for all 3 devm_ calls above?