[alsa-devel] [PATCH] ASoC: Intel: bytcr_5640.c:Refactored if statement and removed buffer
From: Nariman Etemadi narimantos@gmail.com
in function snd_byt_rt5640_mc_probe and removed buffer yt_rt5640_codec_aif_name & byt_rt5640_cpu_dai_name
Signed-off-by: Nariman Etemadi narimantos@gmail.com --- sound/soc/intel/boards/bytcr_rt5640.c | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index 940eb27158da..0d91642ca287 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -1075,8 +1075,6 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = {
/* SoC card */ static char byt_rt5640_codec_name[SND_ACPI_I2C_ID_LEN]; -static char byt_rt5640_codec_aif_name[12]; /* = "rt5640-aif[1|2]" */ -static char byt_rt5640_cpu_dai_name[10]; /* = "ssp[0|2]-port" */ static char byt_rt5640_long_name[40]; /* = "bytcr-rt5640-*-spk-*-mic" */
static int byt_rt5640_suspend(struct snd_soc_card *card) @@ -1268,28 +1266,12 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) log_quirks(&pdev->dev);
if ((byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) || - (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) { - - /* fixup codec aif name */ - snprintf(byt_rt5640_codec_aif_name, - sizeof(byt_rt5640_codec_aif_name), - "%s", "rt5640-aif2"); - - byt_rt5640_dais[dai_index].codec_dai_name = - byt_rt5640_codec_aif_name; - } + (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) + byt_rt5640_dais[dai_index].codec_dai_name = "rt5640-aif2";
if ((byt_rt5640_quirk & BYT_RT5640_SSP0_AIF1) || - (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) { - - /* fixup cpu dai name name */ - snprintf(byt_rt5640_cpu_dai_name, - sizeof(byt_rt5640_cpu_dai_name), - "%s", "ssp0-port"); - - byt_rt5640_dais[dai_index].cpu_dai_name = - byt_rt5640_cpu_dai_name; - } + (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) + byt_rt5640_dais[dai_index].cpu_dai_name = "ssp0-port";
if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) { priv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
From: Damian van Soelen dj.vsoelen@gmail.com
The snprintf calls filling cht_rt5645_cpu_dai_name / cht_rt5645_codec_aif_name always fill them with the same string ("ssp0-port" resp "rt5645-aif2") so instead of keeping these buffers around and making the cpu_dai_name / codec_aif_name point to this, simply update the foo_dai_name and foo_aif_name pointers to directly point to a string constant containing the desired string.
Signed-off-by: Damian van Soelen dj.vsoelen@gmail.com --- sound/soc/intel/boards/cht_bsw_rt5645.c | 26 ++++--------------------- 1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index cbc2d458483f..b15459e56665 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -506,8 +506,6 @@ static struct cht_acpi_card snd_soc_cards[] = { };
static char cht_rt5645_codec_name[SND_ACPI_I2C_ID_LEN]; -static char cht_rt5645_codec_aif_name[12]; /* = "rt5645-aif[1|2]" */ -static char cht_rt5645_cpu_dai_name[10]; /* = "ssp[0|2]-port" */
static bool is_valleyview(void) { @@ -641,28 +639,12 @@ static int snd_cht_mc_probe(struct platform_device *pdev) log_quirks(&pdev->dev);
if ((cht_rt5645_quirk & CHT_RT5645_SSP2_AIF2) || - (cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) { - - /* fixup codec aif name */ - snprintf(cht_rt5645_codec_aif_name, - sizeof(cht_rt5645_codec_aif_name), - "%s", "rt5645-aif2"); - - cht_dailink[dai_index].codec_dai_name = - cht_rt5645_codec_aif_name; - } + (cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) + cht_dailink[dai_index].codec_dai_name = "rt5645-aif2";
if ((cht_rt5645_quirk & CHT_RT5645_SSP0_AIF1) || - (cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) { - - /* fixup cpu dai name name */ - snprintf(cht_rt5645_cpu_dai_name, - sizeof(cht_rt5645_cpu_dai_name), - "%s", "ssp0-port"); - - cht_dailink[dai_index].cpu_dai_name = - cht_rt5645_cpu_dai_name; - } + (cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) + cht_dailink[dai_index].cpu_dai_name = "ssp0-port";
/* override plaform name, if required */ platform_name = mach->mach_params.platform;
On 5/4/19 10:16 AM, Nariman wrote:
From: Damian van Soelen dj.vsoelen@gmail.com
The snprintf calls filling cht_rt5645_cpu_dai_name / cht_rt5645_codec_aif_name always fill them with the same string ("ssp0-port" resp "rt5645-aif2") so instead of keeping these buffers around and making the cpu_dai_name / codec_aif_name point to this, simply update the foo_dai_name and foo_aif_name pointers to directly point to a string constant containing the desired string.
Signed-off-by: Damian van Soelen dj.vsoelen@gmail.com
Need Nariman's Signoff-of-by tag here.
sound/soc/intel/boards/cht_bsw_rt5645.c | 26 ++++--------------------- 1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index cbc2d458483f..b15459e56665 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -506,8 +506,6 @@ static struct cht_acpi_card snd_soc_cards[] = { };
static char cht_rt5645_codec_name[SND_ACPI_I2C_ID_LEN]; -static char cht_rt5645_codec_aif_name[12]; /* = "rt5645-aif[1|2]" */ -static char cht_rt5645_cpu_dai_name[10]; /* = "ssp[0|2]-port" */
static bool is_valleyview(void) { @@ -641,28 +639,12 @@ static int snd_cht_mc_probe(struct platform_device *pdev) log_quirks(&pdev->dev);
if ((cht_rt5645_quirk & CHT_RT5645_SSP2_AIF2) ||
(cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) {/* fixup codec aif name */snprintf(cht_rt5645_codec_aif_name,sizeof(cht_rt5645_codec_aif_name),"%s", "rt5645-aif2");cht_dailink[dai_index].codec_dai_name =cht_rt5645_codec_aif_name;- }
(cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2))cht_dailink[dai_index].codec_dai_name = "rt5645-aif2";
same, not equivalent. SSP2_AIF2 is not handled.
if ((cht_rt5645_quirk & CHT_RT5645_SSP0_AIF1) ||
(cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) {/* fixup cpu dai name name */snprintf(cht_rt5645_cpu_dai_name,sizeof(cht_rt5645_cpu_dai_name),"%s", "ssp0-port");cht_dailink[dai_index].cpu_dai_name =cht_rt5645_cpu_dai_name;- }
(cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2))cht_dailink[dai_index].cpu_dai_name = "ssp0-port";
and same here, SSP0_AIF1 will no longer work.
/* override plaform name, if required */ platform_name = mach->mach_params.platform;
Hi,
On 06-05-19 17:24, Pierre-Louis Bossart wrote:
On 5/4/19 10:16 AM, Nariman wrote:
From: Damian van Soelen dj.vsoelen@gmail.com
The snprintf calls filling cht_rt5645_cpu_dai_name / cht_rt5645_codec_aif_name always fill them with the same string ("ssp0-port" resp "rt5645-aif2") so instead of keeping these buffers around and making the cpu_dai_name / codec_aif_name point to this, simply update the foo_dai_name and foo_aif_name pointers to directly point to a string constant containing the desired string.
Signed-off-by: Damian van Soelen dj.vsoelen@gmail.com
Need Nariman's Signoff-of-by tag here.
Yes, Nariman can you resend with your S-o-b added once the other part of this discussion is resolved?
Note you need to pass
--subject-prefix="PATCH v2"
To git send-email when sending out the new version with your S-o-b added to the 3 patches which you are not the author of.
sound/soc/intel/boards/cht_bsw_rt5645.c | 26 ++++--------------------- 1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index cbc2d458483f..b15459e56665 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -506,8 +506,6 @@ static struct cht_acpi_card snd_soc_cards[] = { }; static char cht_rt5645_codec_name[SND_ACPI_I2C_ID_LEN]; -static char cht_rt5645_codec_aif_name[12]; /* = "rt5645-aif[1|2]" */ -static char cht_rt5645_cpu_dai_name[10]; /* = "ssp[0|2]-port" */ static bool is_valleyview(void) { @@ -641,28 +639,12 @@ static int snd_cht_mc_probe(struct platform_device *pdev) log_quirks(&pdev->dev); if ((cht_rt5645_quirk & CHT_RT5645_SSP2_AIF2) || - (cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) {
- /* fixup codec aif name */ - snprintf(cht_rt5645_codec_aif_name, - sizeof(cht_rt5645_codec_aif_name), - "%s", "rt5645-aif2");
- cht_dailink[dai_index].codec_dai_name = - cht_rt5645_codec_aif_name; - } + (cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) + cht_dailink[dai_index].codec_dai_name = "rt5645-aif2";
same, not equivalent. SSP2_AIF2 is not handled.
Same remark from me, and also the same for the 3th patch you've responded to.
Regards,
Hans
if ((cht_rt5645_quirk & CHT_RT5645_SSP0_AIF1) || - (cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) {
- /* fixup cpu dai name name */ - snprintf(cht_rt5645_cpu_dai_name, - sizeof(cht_rt5645_cpu_dai_name), - "%s", "ssp0-port");
- cht_dailink[dai_index].cpu_dai_name = - cht_rt5645_cpu_dai_name; - } + (cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) + cht_dailink[dai_index].cpu_dai_name = "ssp0-port";
and same here, SSP0_AIF1 will no longer work.
/* override plaform name, if required */ platform_name = mach->mach_params.platform;
From: Erik Bussing eabbussing@outlook.com
Remove code duplication in byt_rt5640_codec_fixup
Signed-off-by: Erik Bussing eabbussing@outlook.com --- sound/soc/intel/boards/bytcr_rt5640.c | 64 ++++++++++----------------- 1 file changed, 23 insertions(+), 41 deletions(-)
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index a22366ce33c4..679be55418bf 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -939,6 +939,7 @@ static int byt_rt5640_codec_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); int ret; + int bits;
/* The DSP will covert the FE rate to 48k, stereo */ rate->min = rate->max = 48000; @@ -949,53 +950,34 @@ static int byt_rt5640_codec_fixup(struct snd_soc_pcm_runtime *rtd,
/* set SSP0 to 16-bit */ params_set_format(params, SNDRV_PCM_FORMAT_S16_LE); - - /* - * Default mode for SSP configuration is TDM 4 slot, override config - * with explicit setting to I2S 2ch 16-bit. The word length is set with - * dai_set_tdm_slot() since there is no other API exposed - */ - ret = snd_soc_dai_set_fmt(rtd->cpu_dai, - SND_SOC_DAIFMT_I2S | - SND_SOC_DAIFMT_NB_NF | - SND_SOC_DAIFMT_CBS_CFS - ); - if (ret < 0) { - dev_err(rtd->dev, "can't set format to I2S, err %d\n", ret); - return ret; - } - - ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, 16); - if (ret < 0) { - dev_err(rtd->dev, "can't set I2S config, err %d\n", ret); - return ret; - } - + bits = 16; } else {
/* set SSP2 to 24-bit */ params_set_format(params, SNDRV_PCM_FORMAT_S24_LE); + bits = 24; + }
- /* - * Default mode for SSP configuration is TDM 4 slot, override config - * with explicit setting to I2S 2ch 24-bit. The word length is set with - * dai_set_tdm_slot() since there is no other API exposed - */ - ret = snd_soc_dai_set_fmt(rtd->cpu_dai, - SND_SOC_DAIFMT_I2S | - SND_SOC_DAIFMT_NB_NF | - SND_SOC_DAIFMT_CBS_CFS - ); - if (ret < 0) { - dev_err(rtd->dev, "can't set format to I2S, err %d\n", ret); - return ret; - }
- ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, 24); - if (ret < 0) { - dev_err(rtd->dev, "can't set I2S config, err %d\n", ret); - return ret; - } + /* + * Default mode for SSP configuration is TDM 4 slot, override config + * with explicit setting to I2S 2ch 24-bit. The word length is set with + * dai_set_tdm_slot() since there is no other API exposed + */ + ret = snd_soc_dai_set_fmt(rtd->cpu_dai, + SND_SOC_DAIFMT_I2S | + SND_SOC_DAIFMT_NB_NF | + SND_SOC_DAIFMT_CBS_CFS + ); + if (ret < 0) { + dev_err(rtd->dev, "can't set format to I2S, err %d\n", ret); + return ret; + } + + ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, bits); + if (ret < 0) { + dev_err(rtd->dev, "can't set I2S config, err %d\n", ret); + return ret; } return 0; }
From: Jordy Ubink jordyubink@hotmail.nl
The snprintf calls filling byt_rt5651_cpu_dai_name / byt_rt5651_cpu_dai_name always fill them with the same string (ssp0-port" resp "rt5651-aif2"). So instead of keeping these buffers around and making the cpu_dai_name / codec_dai_name point to this, simply update the foo_dai_name pointers to directly point to a string constant containing the desired string.
Signed-off-by: Jordy Ubink jordyubink@hotmail.nl --- sound/soc/intel/boards/bytcr_rt5651.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c index e528995668b7..2e1bf43820d8 100644 --- a/sound/soc/intel/boards/bytcr_rt5651.c +++ b/sound/soc/intel/boards/bytcr_rt5651.c @@ -730,8 +730,6 @@ static struct snd_soc_dai_link byt_rt5651_dais[] = {
/* SoC card */ static char byt_rt5651_codec_name[SND_ACPI_I2C_ID_LEN]; -static char byt_rt5651_codec_aif_name[12]; /* = "rt5651-aif[1|2]" */ -static char byt_rt5651_cpu_dai_name[10]; /* = "ssp[0|2]-port" */ static char byt_rt5651_long_name[50]; /* = "bytcr-rt5651-*-spk-*-mic[-swapped-hp]" */
static int byt_rt5651_suspend(struct snd_soc_card *card) @@ -1009,26 +1007,12 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev) log_quirks(&pdev->dev);
if ((byt_rt5651_quirk & BYT_RT5651_SSP2_AIF2) || - (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) { - /* fixup codec aif name */ - snprintf(byt_rt5651_codec_aif_name, - sizeof(byt_rt5651_codec_aif_name), - "%s", "rt5651-aif2"); - - byt_rt5651_dais[dai_index].codec_dai_name = - byt_rt5651_codec_aif_name; - } + (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) + byt_rt5651_dais[dai_index].codec_dai_name = "rt5651-aif2";
if ((byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1) || - (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) { - /* fixup cpu dai name name */ - snprintf(byt_rt5651_cpu_dai_name, - sizeof(byt_rt5651_cpu_dai_name), - "%s", "ssp0-port"); - - byt_rt5651_dais[dai_index].cpu_dai_name = - byt_rt5651_cpu_dai_name; - } + (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) + byt_rt5651_dais[dai_index].cpu_dai_name = "ssp0-port";
if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) { priv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
On Sat, 04 May 2019 17:16:52 +0200, Nariman wrote:
From: Jordy Ubink jordyubink@hotmail.nl
The snprintf calls filling byt_rt5651_cpu_dai_name / byt_rt5651_cpu_dai_name always fill them with the same string (ssp0-port" resp "rt5651-aif2"). So instead of keeping these buffers around and making the cpu_dai_name / codec_dai_name point to this, simply update the foo_dai_name pointers to directly point to a string constant containing the desired string.
Signed-off-by: Jordy Ubink jordyubink@hotmail.nl
If you submit a patch, please give your own sign-off as well as the author's one, even if the patch is not written by you. This is a legal requirement.
thanks,
Takashi
Hi,
On 05-05-19 09:51, Takashi Iwai wrote:
On Sat, 04 May 2019 17:16:52 +0200, Nariman wrote:
From: Jordy Ubink jordyubink@hotmail.nl
The snprintf calls filling byt_rt5651_cpu_dai_name / byt_rt5651_cpu_dai_name always fill them with the same string (ssp0-port" resp "rt5651-aif2"). So instead of keeping these buffers around and making the cpu_dai_name / codec_dai_name point to this, simply update the foo_dai_name pointers to directly point to a string constant containing the desired string.
Signed-off-by: Jordy Ubink jordyubink@hotmail.nl
If you submit a patch, please give your own sign-off as well as the author's one, even if the patch is not written by you. This is a legal requirement.
Sorry, that is my bad, Nariman and the author authors of the patches are a group of students doing some kernel work for me and this is a warm-up assignment for them to get used to the kernel development process.
I forgot to point out to Nariman that since he is sending out the entire series for all 4 of them, he needs to add his S-o-b.
Regards,
Hans
On 5/6/19 10:40 AM, Hans de Goede wrote:
Hi,
On 05-05-19 09:51, Takashi Iwai wrote:
On Sat, 04 May 2019 17:16:52 +0200, Nariman wrote:
From: Jordy Ubink jordyubink@hotmail.nl
The snprintf calls filling byt_rt5651_cpu_dai_name / byt_rt5651_cpu_dai_name always fill them with the same string (ssp0-port" resp "rt5651-aif2"). So instead of keeping these buffers around and making the cpu_dai_name / codec_dai_name point to this, simply update the foo_dai_name pointers to directly point to a string constant containing the desired string.
Signed-off-by: Jordy Ubink jordyubink@hotmail.nl
If you submit a patch, please give your own sign-off as well as the author's one, even if the patch is not written by you. This is a legal requirement.
Sorry, that is my bad, Nariman and the author authors of the patches are a group of students doing some kernel work for me and this is a warm-up assignment for them to get used to the kernel development process.
I forgot to point out to Nariman that since he is sending out the entire series for all 4 of them, he needs to add his S-o-b.
One suggestion for additional cleanups: can we try and remove all these static variables and move them to the context structure? It's been on my TODO list for a while.
On 5/4/19 10:16 AM, Nariman wrote:
From: Jordy Ubink jordyubink@hotmail.nl
The snprintf calls filling byt_rt5651_cpu_dai_name / byt_rt5651_cpu_dai_name always fill them with the same string (ssp0-port" resp "rt5651-aif2"). So instead of keeping these buffers around and making the cpu_dai_name / codec_dai_name point to this, simply update the foo_dai_name pointers to directly point to a string constant containing the desired string.
Signed-off-by: Jordy Ubink jordyubink@hotmail.nl
sound/soc/intel/boards/bytcr_rt5651.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c index e528995668b7..2e1bf43820d8 100644 --- a/sound/soc/intel/boards/bytcr_rt5651.c +++ b/sound/soc/intel/boards/bytcr_rt5651.c @@ -730,8 +730,6 @@ static struct snd_soc_dai_link byt_rt5651_dais[] = {
/* SoC card */ static char byt_rt5651_codec_name[SND_ACPI_I2C_ID_LEN]; -static char byt_rt5651_codec_aif_name[12]; /* = "rt5651-aif[1|2]" */ -static char byt_rt5651_cpu_dai_name[10]; /* = "ssp[0|2]-port" */ static char byt_rt5651_long_name[50]; /* = "bytcr-rt5651-*-spk-*-mic[-swapped-hp]" */
static int byt_rt5651_suspend(struct snd_soc_card *card) @@ -1009,26 +1007,12 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev) log_quirks(&pdev->dev);
if ((byt_rt5651_quirk & BYT_RT5651_SSP2_AIF2) ||
(byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) {/* fixup codec aif name */snprintf(byt_rt5651_codec_aif_name,sizeof(byt_rt5651_codec_aif_name),"%s", "rt5651-aif2");byt_rt5651_dais[dai_index].codec_dai_name =byt_rt5651_codec_aif_name;- }
(byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2))byt_rt5651_dais[dai_index].codec_dai_name = "rt5651-aif2";if ((byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1) ||
(byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) {/* fixup cpu dai name name */snprintf(byt_rt5651_cpu_dai_name,sizeof(byt_rt5651_cpu_dai_name),"%s", "ssp0-port");byt_rt5651_dais[dai_index].cpu_dai_name =byt_rt5651_cpu_dai_name;- }
(byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2))byt_rt5651_dais[dai_index].cpu_dai_name = "ssp0-port";
same issues with Signed-off-by and missing quirks.
if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) { priv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
static int byt_rt5640_suspend(struct snd_soc_card *card) @@ -1268,28 +1266,12 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) log_quirks(&pdev->dev);
if ((byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) ||
(byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) {/* fixup codec aif name */snprintf(byt_rt5640_codec_aif_name,sizeof(byt_rt5640_codec_aif_name),"%s", "rt5640-aif2");byt_rt5640_dais[dai_index].codec_dai_name =byt_rt5640_codec_aif_name;- }
(byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2))byt_rt5640_dais[dai_index].codec_dai_name = "rt5640-aif2";
This is not equivalent, you don't deal with the (byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) case. The default is SSP_AIF1
if ((byt_rt5640_quirk & BYT_RT5640_SSP0_AIF1) ||
(byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) {/* fixup cpu dai name name */snprintf(byt_rt5640_cpu_dai_name,sizeof(byt_rt5640_cpu_dai_name),"%s", "ssp0-port");byt_rt5640_dais[dai_index].cpu_dai_name =byt_rt5640_cpu_dai_name;- }
(byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2))byt_rt5640_dais[dai_index].cpu_dai_name = "ssp0-port";
Same here, this is not equivalent. the SSP0_AIF1 case is not handled. it's fine to remove the intermediate buffers, but you can't remove support for 2 out of the 4 combinations supported.
Hi Pierre-Louis,
Nariman and the author authors of these patches are a group of students doing some kernel work for me and this is a warm-up assignment for them to get used to the kernel development process.
On 06-05-19 17:21, Pierre-Louis Bossart wrote:
static int byt_rt5640_suspend(struct snd_soc_card *card) @@ -1268,28 +1266,12 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) log_quirks(&pdev->dev); if ((byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) || - (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) {
- /* fixup codec aif name */ - snprintf(byt_rt5640_codec_aif_name, - sizeof(byt_rt5640_codec_aif_name), - "%s", "rt5640-aif2");
- byt_rt5640_dais[dai_index].codec_dai_name = - byt_rt5640_codec_aif_name; - } + (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) + byt_rt5640_dais[dai_index].codec_dai_name = "rt5640-aif2";
This is not equivalent, you don't deal with the (byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) case. The default is SSP_AIF1
I might be mistaken here, but look closer, the original: if ((byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) ||
Line is kept, so the new code block is:
if ((byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) || (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) byt_rt5640_dais[dai_index].codec_dai_name = "rt5640-aif2";
Which does take the BYT_RT5640_SSP2_AIF2 into account.
if ((byt_rt5640_quirk & BYT_RT5640_SSP0_AIF1) || - (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) {
- /* fixup cpu dai name name */ - snprintf(byt_rt5640_cpu_dai_name, - sizeof(byt_rt5640_cpu_dai_name), - "%s", "ssp0-port");
- byt_rt5640_dais[dai_index].cpu_dai_name = - byt_rt5640_cpu_dai_name; - } + (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) + byt_rt5640_dais[dai_index].cpu_dai_name = "ssp0-port";
Same here, this is not equivalent. the SSP0_AIF1 case is not handled. it's fine to remove the intermediate buffers, but you can't remove support for 2 out of the 4 combinations supported.
Same remark here from me too :)
Regards,
Hans
On 5/6/19 10:43 AM, Hans de Goede wrote:
Hi Pierre-Louis,
Nariman and the author authors of these patches are a group of students doing some kernel work for me and this is a warm-up assignment for them to get used to the kernel development process.
On 06-05-19 17:21, Pierre-Louis Bossart wrote:
static int byt_rt5640_suspend(struct snd_soc_card *card) @@ -1268,28 +1266,12 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) log_quirks(&pdev->dev); if ((byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) || - (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) {
- /* fixup codec aif name */ - snprintf(byt_rt5640_codec_aif_name, - sizeof(byt_rt5640_codec_aif_name), - "%s", "rt5640-aif2");
- byt_rt5640_dais[dai_index].codec_dai_name = - byt_rt5640_codec_aif_name; - } + (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) + byt_rt5640_dais[dai_index].codec_dai_name = "rt5640-aif2";
This is not equivalent, you don't deal with the (byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) case. The default is SSP_AIF1
I might be mistaken here, but look closer, the original: if ((byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) ||
Line is kept, so the new code block is:
if ((byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) || (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) byt_rt5640_dais[dai_index].codec_dai_name = "rt5640-aif2";
Which does take the BYT_RT5640_SSP2_AIF2 into account.
Ah yes, my mistake. Looks good then.
if ((byt_rt5640_quirk & BYT_RT5640_SSP0_AIF1) || - (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) {
- /* fixup cpu dai name name */ - snprintf(byt_rt5640_cpu_dai_name, - sizeof(byt_rt5640_cpu_dai_name), - "%s", "ssp0-port");
- byt_rt5640_dais[dai_index].cpu_dai_name = - byt_rt5640_cpu_dai_name; - } + (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) + byt_rt5640_dais[dai_index].cpu_dai_name = "ssp0-port";
Same here, this is not equivalent. the SSP0_AIF1 case is not handled. it's fine to remove the intermediate buffers, but you can't remove support for 2 out of the 4 combinations supported.
Same remark here from me too :)
Regards,
Hans
Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (4)
-
Hans de Goede -
Nariman -
Pierre-Louis Bossart -
Takashi Iwai