All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Baytrail-Cherrytrail updates
@ 2017-10-05 18:52 Pierre-Louis Bossart
  2017-10-05 18:52 ` [PATCH 1/6] ASoC: Intel: bytcr_rt5651: add MCLK support and fix quirks Pierre-Louis Bossart
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Pierre-Louis Bossart @ 2017-10-05 18:52 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, vinod.koul, broonie, Pierre-Louis Bossart, Andy Shevchenko

As discussed on the alsa-devel mailing list, here is a set of patches to fix

1. previous feedback on bytcht_rt5651 - no new functionality beyond
what was already reviewed
2. cosmetic issues (variables in reverse xmas tree order, etc)
3. unnecessary clock tests

Since the changes are very small, I grouped them by machine
driver instead of generating more patches. let me know if a different
split is needed.

The last patch addresses Andy's comment that we should not use
strncmp(a,b,strlen(b)) when strcmp(a,b) is fine. This change is
applied across all machine drivers.

Pierre-Louis Bossart (6):
  ASoC: Intel: bytcr_rt5651: add MCLK support and fix quirks
  ASoC: Intel: bytcr_rt5651: filter codec name and fix variable
    declarations
  ASoC: Intel: bytcr_rt5640: cosmetic fixes
  ASoC: Intel: cht_bsw_rt5645: cosmetic fixes
  ASoC: Intel: bytcht_da7213: cosmetic fixes
  ASoC: Intel: boards: use strcmp with constant literals

 sound/soc/intel/boards/bxt_da7219_max98357a.c   |   3 +-
 sound/soc/intel/boards/bytcht_da7213.c          |   6 +-
 sound/soc/intel/boards/bytcht_es8316.c          |   3 +-
 sound/soc/intel/boards/bytcr_rt5640.c           |  79 ++++----
 sound/soc/intel/boards/bytcr_rt5651.c           | 257 ++++++++++++++++++++++--
 sound/soc/intel/boards/cht_bsw_max98090_ti.c    |   3 +-
 sound/soc/intel/boards/cht_bsw_rt5645.c         |  79 ++++----
 sound/soc/intel/boards/cht_bsw_rt5672.c         |   3 +-
 sound/soc/intel/boards/skl_nau88l25_max98357a.c |   3 +-
 sound/soc/intel/boards/skl_nau88l25_ssm4567.c   |   3 +-
 10 files changed, 324 insertions(+), 115 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/6] ASoC: Intel: bytcr_rt5651: add MCLK support and fix quirks
  2017-10-05 18:52 [PATCH 0/6] Baytrail-Cherrytrail updates Pierre-Louis Bossart
@ 2017-10-05 18:52 ` Pierre-Louis Bossart
  2017-10-05 18:52 ` [PATCH 2/6] ASoC: Intel: bytcr_rt5651: filter codec name and fix variable declarations Pierre-Louis Bossart
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Pierre-Louis Bossart @ 2017-10-05 18:52 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, vinod.koul, broonie, Pierre-Louis Bossart, Andy Shevchenko

Same as for other codecs, enable MCLK by default. When it is not
present, e.g. on MinnowBoard B3 since it's not routed on the LSE
connector, we fall back to blck-based clocking.

The DMIC quirks are also fixed, there is a single DMIC input of the
codec.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 230 +++++++++++++++++++++++++++++++---
 1 file changed, 214 insertions(+), 16 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 441f735a198f..e41c7ea91bd9 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -21,9 +21,11 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/acpi.h>
+#include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
+#include <asm/platform_sst_audio.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
@@ -31,14 +33,119 @@
 #include "../../codecs/rt5651.h"
 #include "../atom/sst-atom-controls.h"
 
+enum {
+	BYT_RT5651_DMIC_MAP,
+	BYT_RT5651_IN1_MAP,
+};
+
+#define BYT_RT5651_MAP(quirk)	((quirk) & GENMASK(7, 0))
+#define BYT_RT5651_DMIC_EN	BIT(16)
+#define BYT_RT5651_MCLK_EN	BIT(17)
+#define BYT_RT5651_MCLK_25MHZ	BIT(18)
+
+struct byt_rt5651_private {
+	struct clk *mclk;
+};
+
+static unsigned long byt_rt5651_quirk = BYT_RT5651_DMIC_MAP |
+					BYT_RT5651_DMIC_EN |
+					BYT_RT5651_MCLK_EN;
+
+static void log_quirks(struct device *dev)
+{
+	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_DMIC_MAP)
+		dev_info(dev, "quirk DMIC_MAP enabled");
+	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN1_MAP)
+		dev_info(dev, "quirk IN1_MAP enabled");
+	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
+		dev_info(dev, "quirk DMIC enabled");
+	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN)
+		dev_info(dev, "quirk MCLK_EN enabled");
+	if (byt_rt5651_quirk & BYT_RT5651_MCLK_25MHZ)
+		dev_info(dev, "quirk MCLK_25MHZ enabled");
+}
+
+#define BYT_CODEC_DAI1	"rt5651-aif1"
+
+static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card)
+{
+	struct snd_soc_pcm_runtime *rtd;
+
+	list_for_each_entry(rtd, &card->rtd_list, list) {
+		if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI1,
+			     strlen(BYT_CODEC_DAI1)))
+			return rtd->codec_dai;
+	}
+	return NULL;
+}
+
+static int platform_clock_control(struct snd_soc_dapm_widget *w,
+				  struct snd_kcontrol *k, int  event)
+{
+	struct snd_soc_dapm_context *dapm = w->dapm;
+	struct snd_soc_card *card = dapm->card;
+	struct snd_soc_dai *codec_dai;
+	struct byt_rt5651_private *priv = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	codec_dai = byt_get_codec_dai(card);
+	if (!codec_dai) {
+		dev_err(card->dev,
+			"Codec dai not found; Unable to set platform clock\n");
+		return -EIO;
+	}
+
+	if (SND_SOC_DAPM_EVENT_ON(event)) {
+		if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) {
+			ret = clk_prepare_enable(priv->mclk);
+			if (ret < 0) {
+				dev_err(card->dev,
+					"could not configure MCLK state");
+				return ret;
+			}
+		}
+		ret = snd_soc_dai_set_sysclk(codec_dai, RT5651_SCLK_S_PLL1,
+					     48000 * 512,
+					     SND_SOC_CLOCK_IN);
+	} else {
+		/*
+		 * Set codec clock source to internal clock before
+		 * turning off the platform clock. Codec needs clock
+		 * for Jack detection and button press
+		 */
+		ret = snd_soc_dai_set_sysclk(codec_dai, RT5651_SCLK_S_RCCLK,
+					     48000 * 512,
+					     SND_SOC_CLOCK_IN);
+		if (!ret)
+			if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN)
+				clk_disable_unprepare(priv->mclk);
+	}
+
+	if (ret < 0) {
+		dev_err(card->dev, "can't set codec sysclk: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static const struct snd_soc_dapm_widget byt_rt5651_widgets[] = {
 	SND_SOC_DAPM_HP("Headphone", NULL),
 	SND_SOC_DAPM_MIC("Headset Mic", NULL),
 	SND_SOC_DAPM_MIC("Internal Mic", NULL),
 	SND_SOC_DAPM_SPK("Speaker", NULL),
+	SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
+			    platform_clock_control, SND_SOC_DAPM_PRE_PMU |
+			    SND_SOC_DAPM_POST_PMD),
+
 };
 
 static const struct snd_soc_dapm_route byt_rt5651_audio_map[] = {
+	{"Headphone", NULL, "Platform Clock"},
+	{"Headset Mic", NULL, "Platform Clock"},
+	{"Internal Mic", NULL, "Platform Clock"},
+	{"Speaker", NULL, "Platform Clock"},
+
 	{"AIF1 Playback", NULL, "ssp2 Tx"},
 	{"ssp2 Tx", NULL, "codec_out0"},
 	{"ssp2 Tx", NULL, "codec_out1"},
@@ -64,18 +171,6 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_map[] = {
 	{"IN1P", NULL, "Internal Mic"},
 };
 
-enum {
-	BYT_RT5651_DMIC1_MAP,
-	BYT_RT5651_DMIC2_MAP,
-	BYT_RT5651_IN1_MAP,
-};
-
-#define BYT_RT5651_MAP(quirk)	((quirk) & 0xff)
-#define BYT_RT5651_DMIC_EN	BIT(16)
-
-static unsigned long byt_rt5651_quirk = BYT_RT5651_DMIC1_MAP |
-					BYT_RT5651_DMIC_EN;
-
 static const struct snd_kcontrol_new byt_rt5651_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Headphone"),
 	SOC_DAPM_PIN_SWITCH("Headset Mic"),
@@ -100,9 +195,26 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	ret = snd_soc_dai_set_pll(codec_dai, 0, RT5651_PLL1_S_BCLK1,
-				  params_rate(params) * 50,
-				  params_rate(params) * 512);
+	if (!(byt_rt5651_quirk & BYT_RT5651_MCLK_EN)) {
+		/* 2x25 bit slots on SSP2 */
+		ret = snd_soc_dai_set_pll(codec_dai, 0,
+					RT5651_PLL1_S_BCLK1,
+					params_rate(params) * 50,
+					params_rate(params) * 512);
+	} else {
+		if (byt_rt5651_quirk & BYT_RT5651_MCLK_25MHZ) {
+			ret = snd_soc_dai_set_pll(codec_dai, 0,
+						RT5651_PLL1_S_MCLK,
+						25000000,
+						params_rate(params) * 512);
+		} else {
+			ret = snd_soc_dai_set_pll(codec_dai, 0,
+						RT5651_PLL1_S_MCLK,
+						19200000,
+						params_rate(params) * 512);
+		}
+	}
+
 	if (ret < 0) {
 		dev_err(rtd->dev, "can't set codec pll: %d\n", ret);
 		return ret;
@@ -111,7 +223,22 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+static int byt_rt5651_quirk_cb(const struct dmi_system_id *id)
+{
+	byt_rt5651_quirk = (unsigned long)id->driver_data;
+	return 1;
+}
+
 static const struct dmi_system_id byt_rt5651_quirk_table[] = {
+	{
+		.callback = byt_rt5651_quirk_cb,
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Circuitco"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Minnowboard Max B3 PLATFORM"),
+		},
+		.driver_data = (void *)(BYT_RT5651_DMIC_MAP |
+					BYT_RT5651_DMIC_EN),
+	},
 	{}
 };
 
@@ -120,11 +247,11 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 	int ret;
 	struct snd_soc_card *card = runtime->card;
 	const struct snd_soc_dapm_route *custom_map;
+	struct byt_rt5651_private *priv = snd_soc_card_get_drvdata(card);
 	int num_routes;
 
 	card->dapm.idle_bias_off = true;
 
-	dmi_check_system(byt_rt5651_quirk_table);
 	switch (BYT_RT5651_MAP(byt_rt5651_quirk)) {
 	case BYT_RT5651_IN1_MAP:
 		custom_map = byt_rt5651_intmic_in1_map;
@@ -147,6 +274,30 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 	snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone");
 	snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker");
 
+	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) {
+		/*
+		 * The firmware might enable the clock at
+		 * boot (this information may or may not
+		 * be reflected in the enable clock register).
+		 * To change the rate we must disable the clock
+		 * first to cover these cases. Due to common
+		 * clock framework restrictions that do not allow
+		 * to disable a clock that has not been enabled,
+		 * we need to enable the clock first.
+		 */
+		ret = clk_prepare_enable(priv->mclk);
+		if (!ret)
+			clk_disable_unprepare(priv->mclk);
+
+		if (byt_rt5651_quirk & BYT_RT5651_MCLK_25MHZ)
+			ret = clk_set_rate(priv->mclk, 25000000);
+		else
+			ret = clk_set_rate(priv->mclk, 19200000);
+
+		if (ret)
+			dev_err(card->dev, "unable to set MCLK rate\n");
+	}
+
 	return ret;
 }
 
@@ -295,10 +446,57 @@ static struct snd_soc_card byt_rt5651_card = {
 static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 {
 	int ret_val = 0;
+	struct byt_rt5651_private *priv;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC);
+	if (!priv)
+		return -ENOMEM;
 
 	/* register the soc card */
 	byt_rt5651_card.dev = &pdev->dev;
 
+	mach = byt_rt5651_card.dev->platform_data;
+	snd_soc_card_set_drvdata(&byt_rt5651_card, priv);
+
+	/* fix index of codec dai */
+	dai_index = MERR_DPCM_COMPR + 1;
+	for (i = 0; i < ARRAY_SIZE(byt_rt5651_dais); i++) {
+		if (!strcmp(byt_rt5651_dais[i].codec_name, "i2c-10EC5651:00")) {
+			dai_index = i;
+			break;
+		}
+	}
+
+	/* fixup codec name based on HID */
+	i2c_name = sst_acpi_find_name_from_hid(mach->id);
+	if (i2c_name != NULL) {
+		snprintf(byt_rt5651_codec_name, sizeof(byt_rt5651_codec_name),
+			"%s%s", "i2c-", i2c_name);
+
+		byt_rt5651_dais[dai_index].codec_name = byt_rt5651_codec_name;
+	}
+
+	/* check quirks before creating card */
+	dmi_check_system(byt_rt5651_quirk_table);
+	log_quirks(&pdev->dev);
+
+	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) {
+		priv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
+		if (IS_ERR(priv->mclk)) {
+			dev_err(&pdev->dev,
+				"Failed to get MCLK from pmc_plt_clk_3: %ld\n",
+				PTR_ERR(priv->mclk));
+			/*
+			 * Fall back to bit clock usage for -ENOENT (clock not
+			 * available likely due to missing dependencies), bail
+			 * for all other errors, including -EPROBE_DEFER
+			 */
+			if (ret_val != -ENOENT)
+				return ret_val;
+			byt_rt5651_quirk &= ~BYT_RT5651_MCLK_EN;
+		}
+	}
+
 	ret_val = devm_snd_soc_register_card(&pdev->dev, &byt_rt5651_card);
 
 	if (ret_val) {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/6] ASoC: Intel: bytcr_rt5651: filter codec name and fix variable declarations
  2017-10-05 18:52 [PATCH 0/6] Baytrail-Cherrytrail updates Pierre-Louis Bossart
  2017-10-05 18:52 ` [PATCH 1/6] ASoC: Intel: bytcr_rt5651: add MCLK support and fix quirks Pierre-Louis Bossart
@ 2017-10-05 18:52 ` Pierre-Louis Bossart
  2017-10-05 18:52 ` [PATCH 3/6] ASoC: Intel: bytcr_rt5640: cosmetic fixes Pierre-Louis Bossart
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Pierre-Louis Bossart @ 2017-10-05 18:52 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, vinod.koul, broonie, Pierre-Louis Bossart, Andy Shevchenko

Use same fix as other codecs to work around BIOS/ACPI issues

Also reorder variables in reverse x-mas tree as suggested by Andy

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index e41c7ea91bd9..1b97aa735747 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -32,6 +32,7 @@
 #include <sound/jack.h>
 #include "../../codecs/rt5651.h"
 #include "../atom/sst-atom-controls.h"
+#include "../common/sst-acpi.h"
 
 enum {
 	BYT_RT5651_DMIC_MAP,
@@ -244,11 +245,11 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 
 static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 {
-	int ret;
 	struct snd_soc_card *card = runtime->card;
-	const struct snd_soc_dapm_route *custom_map;
 	struct byt_rt5651_private *priv = snd_soc_card_get_drvdata(card);
+	const struct snd_soc_dapm_route *custom_map;
 	int num_routes;
+	int ret;
 
 	card->dapm.idle_bias_off = true;
 
@@ -443,10 +444,16 @@ static struct snd_soc_card byt_rt5651_card = {
 	.fully_routed = true,
 };
 
+static char byt_rt5651_codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */
+
 static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 {
-	int ret_val = 0;
 	struct byt_rt5651_private *priv;
+	struct sst_acpi_mach *mach;
+	const char *i2c_name = NULL;
+	int ret_val = 0;
+	int dai_index;
+	int i;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC);
 	if (!priv)
@@ -454,6 +461,25 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 
 	/* register the soc card */
 	byt_rt5651_card.dev = &pdev->dev;
+	mach = byt_rt5651_card.dev->platform_data;
+
+	/* fix index of codec dai */
+	dai_index = MERR_DPCM_COMPR + 1;
+	for (i = 0; i < ARRAY_SIZE(byt_rt5651_dais); i++) {
+		if (!strcmp(byt_rt5651_dais[i].codec_name, "i2c-10EC5651:00")) {
+			dai_index = i;
+			break;
+		}
+	}
+
+	/* fixup codec name based on HID */
+	i2c_name = sst_acpi_find_name_from_hid(mach->id);
+	if (i2c_name) {
+		snprintf(byt_rt5651_codec_name, sizeof(byt_rt5651_codec_name),
+			"%s%s", "i2c-", i2c_name);
+
+		byt_rt5651_dais[dai_index].codec_name = byt_rt5651_codec_name;
+	}
 
 	mach = byt_rt5651_card.dev->platform_data;
 	snd_soc_card_set_drvdata(&byt_rt5651_card, priv);
@@ -469,7 +495,7 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 
 	/* fixup codec name based on HID */
 	i2c_name = sst_acpi_find_name_from_hid(mach->id);
-	if (i2c_name != NULL) {
+	if (i2c_name) {
 		snprintf(byt_rt5651_codec_name, sizeof(byt_rt5651_codec_name),
 			"%s%s", "i2c-", i2c_name);
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/6] ASoC: Intel: bytcr_rt5640: cosmetic fixes
  2017-10-05 18:52 [PATCH 0/6] Baytrail-Cherrytrail updates Pierre-Louis Bossart
  2017-10-05 18:52 ` [PATCH 1/6] ASoC: Intel: bytcr_rt5651: add MCLK support and fix quirks Pierre-Louis Bossart
  2017-10-05 18:52 ` [PATCH 2/6] ASoC: Intel: bytcr_rt5651: filter codec name and fix variable declarations Pierre-Louis Bossart
@ 2017-10-05 18:52 ` Pierre-Louis Bossart
  2017-10-18 11:31   ` Applied "ASoC: Intel: bytcr_rt5640: cosmetic fixes" to the asoc tree Mark Brown
  2017-10-05 18:52 ` [PATCH 4/6] ASoC: Intel: cht_bsw_rt5645: cosmetic fixes Pierre-Louis Bossart
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Pierre-Louis Bossart @ 2017-10-05 18:52 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, vinod.koul, broonie, Pierre-Louis Bossart, Andy Shevchenko

Reorder variable names, change MCLK test, change quirks
No functional change

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 73 +++++++++++++++++------------------
 1 file changed, 36 insertions(+), 37 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 7cee09d08ad7..ffb3ca971df4 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -22,12 +22,12 @@
 #include <linux/moduleparam.h>
 #include <linux/platform_device.h>
 #include <linux/acpi.h>
+#include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
 #include <asm/cpu_device_id.h>
 #include <asm/platform_sst_audio.h>
-#include <linux/clk.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
@@ -44,13 +44,13 @@ enum {
 	BYT_RT5640_IN3_MAP,
 };
 
-#define BYT_RT5640_MAP(quirk)	((quirk) & 0xff)
+#define BYT_RT5640_MAP(quirk)	((quirk) &  GENMASK(7, 0))
 #define BYT_RT5640_DMIC_EN	BIT(16)
 #define BYT_RT5640_MONO_SPEAKER BIT(17)
 #define BYT_RT5640_DIFF_MIC     BIT(18) /* defaut is single-ended */
-#define BYT_RT5640_SSP2_AIF2     BIT(19) /* default is using AIF1  */
-#define BYT_RT5640_SSP0_AIF1     BIT(20)
-#define BYT_RT5640_SSP0_AIF2     BIT(21)
+#define BYT_RT5640_SSP2_AIF2    BIT(19) /* default is using AIF1  */
+#define BYT_RT5640_SSP0_AIF1    BIT(20)
+#define BYT_RT5640_SSP0_AIF2    BIT(21)
 #define BYT_RT5640_MCLK_EN	BIT(22)
 #define BYT_RT5640_MCLK_25MHZ	BIT(23)
 
@@ -178,7 +178,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 	}
 
 	if (SND_SOC_DAPM_EVENT_ON(event)) {
-		if (priv->mclk) {
+		if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
 			ret = clk_prepare_enable(priv->mclk);
 			if (ret < 0) {
 				dev_err(card->dev,
@@ -199,7 +199,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 					     48000 * 512,
 					     SND_SOC_CLOCK_IN);
 		if (!ret) {
-			if (priv->mclk)
+			if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN)
 				clk_disable_unprepare(priv->mclk);
 		}
 	}
@@ -376,8 +376,8 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100TA"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN1_MAP |
-						 BYT_RT5640_MCLK_EN),
+		.driver_data = (void *)(BYT_RT5640_IN1_MAP |
+					BYT_RT5640_MCLK_EN),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -385,12 +385,11 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100TAF"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN1_MAP |
-						 BYT_RT5640_MONO_SPEAKER |
-						 BYT_RT5640_DIFF_MIC |
-						 BYT_RT5640_SSP0_AIF2 |
-						 BYT_RT5640_MCLK_EN
-						 ),
+		.driver_data = (void *)(BYT_RT5640_IN1_MAP |
+					BYT_RT5640_MONO_SPEAKER |
+					BYT_RT5640_DIFF_MIC |
+					BYT_RT5640_SSP0_AIF2 |
+					BYT_RT5640_MCLK_EN),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -398,9 +397,9 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "DellInc."),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Venue 8 Pro 5830"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_DMIC2_MAP |
-						 BYT_RT5640_DMIC_EN |
-						 BYT_RT5640_MCLK_EN),
+		.driver_data = (void *)(BYT_RT5640_DMIC2_MAP |
+					BYT_RT5640_DMIC_EN |
+					BYT_RT5640_MCLK_EN),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -408,8 +407,8 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "HP ElitePad 1000 G2"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN1_MAP |
-						 BYT_RT5640_MCLK_EN),
+		.driver_data = (void *)(BYT_RT5640_IN1_MAP |
+					BYT_RT5640_MCLK_EN),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -417,8 +416,8 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Circuitco"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "Minnowboard Max B3 PLATFORM"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_DMIC1_MAP |
-						 BYT_RT5640_DMIC_EN),
+		.driver_data = (void *)(BYT_RT5640_DMIC1_MAP |
+					BYT_RT5640_DMIC_EN),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -426,9 +425,9 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_MATCH(DMI_BOARD_VENDOR, "TECLAST"),
 			DMI_MATCH(DMI_BOARD_NAME, "tPAD"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN3_MAP |
-						BYT_RT5640_MCLK_EN |
-						BYT_RT5640_SSP0_AIF1),
+		.driver_data = (void *)(BYT_RT5640_IN3_MAP |
+					BYT_RT5640_MCLK_EN |
+					BYT_RT5640_SSP0_AIF1),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -436,7 +435,7 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire SW5-012"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN1_MAP |
+		.driver_data = (void *)(BYT_RT5640_IN1_MAP |
 						 BYT_RT5640_MCLK_EN |
 						 BYT_RT5640_SSP0_AIF1),
 
@@ -446,9 +445,9 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Insyde"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN3_MAP |
-						 BYT_RT5640_MCLK_EN |
-						 BYT_RT5640_SSP0_AIF1),
+		.driver_data = (void *)(BYT_RT5640_IN3_MAP |
+					BYT_RT5640_MCLK_EN |
+					BYT_RT5640_SSP0_AIF1),
 
 	},
 	{}
@@ -456,12 +455,12 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 
 static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 {
-	int ret;
-	struct snd_soc_codec *codec = runtime->codec;
 	struct snd_soc_card *card = runtime->card;
-	const struct snd_soc_dapm_route *custom_map;
 	struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card);
+	struct snd_soc_codec *codec = runtime->codec;
+	const struct snd_soc_dapm_route *custom_map;
 	int num_routes;
+	int ret;
 
 	card->dapm.idle_bias_off = true;
 
@@ -549,7 +548,7 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 	snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone");
 	snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker");
 
-	if (priv->mclk) {
+	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
 		/*
 		 * The firmware might enable the clock at
 		 * boot (this information may or may not
@@ -758,12 +757,12 @@ struct acpi_chan_package {   /* ACPICA seems to require 64 bit integers */
 
 static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 {
-	int ret_val = 0;
+	struct byt_rt5640_private *priv;
 	struct sst_acpi_mach *mach;
 	const char *i2c_name = NULL;
-	int i;
+	int ret_val = 0;
 	int dai_index;
-	struct byt_rt5640_private *priv;
+	int i;
 
 	is_bytcr = false;
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC);
@@ -786,7 +785,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 
 	/* fixup codec name based on HID */
 	i2c_name = sst_acpi_find_name_from_hid(mach->id);
-	if (i2c_name != NULL) {
+	if (i2c_name) {
 		snprintf(byt_rt5640_codec_name, sizeof(byt_rt5640_codec_name),
 			"%s%s", "i2c-", i2c_name);
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/6] ASoC: Intel: cht_bsw_rt5645: cosmetic fixes
  2017-10-05 18:52 [PATCH 0/6] Baytrail-Cherrytrail updates Pierre-Louis Bossart
                   ` (2 preceding siblings ...)
  2017-10-05 18:52 ` [PATCH 3/6] ASoC: Intel: bytcr_rt5640: cosmetic fixes Pierre-Louis Bossart
@ 2017-10-05 18:52 ` Pierre-Louis Bossart
  2017-10-18 11:31   ` Applied "ASoC: Intel: cht_bsw_rt5645: cosmetic fixes" to the asoc tree Mark Brown
  2017-10-05 18:52 ` [PATCH 5/6] ASoC: Intel: bytcht_da7213: cosmetic fixes Pierre-Louis Bossart
  2017-10-05 18:52 ` [PATCH 6/6] ASoC: Intel: boards: use strcmp with constant literals Pierre-Louis Bossart
  5 siblings, 1 reply; 16+ messages in thread
From: Pierre-Louis Bossart @ 2017-10-05 18:52 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, vinod.koul, broonie, Pierre-Louis Bossart, Andy Shevchenko

Reorder variable names, change MCLK test, change quirks
No functional change

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/boards/cht_bsw_rt5645.c | 73 ++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index d553e2b67c92..6ccb599c1082 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -21,13 +21,13 @@
  */
 
 #include <linux/module.h>
-#include <linux/acpi.h>
 #include <linux/platform_device.h>
+#include <linux/acpi.h>
+#include <linux/clk.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
 #include <asm/cpu_device_id.h>
 #include <asm/platform_sst_audio.h>
-#include <linux/clk.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
@@ -53,7 +53,7 @@ struct cht_mc_private {
 	struct clk *mclk;
 };
 
-#define CHT_RT5645_MAP(quirk)	((quirk) & 0xff)
+#define CHT_RT5645_MAP(quirk)	((quirk) & GENMASK(7, 0))
 #define CHT_RT5645_SSP2_AIF2     BIT(16) /* default is using AIF1  */
 #define CHT_RT5645_SSP0_AIF1     BIT(17)
 #define CHT_RT5645_SSP0_AIF2     BIT(18)
@@ -101,13 +101,11 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 	}
 
 	if (SND_SOC_DAPM_EVENT_ON(event)) {
-		if (ctx->mclk) {
-			ret = clk_prepare_enable(ctx->mclk);
-			if (ret < 0) {
-				dev_err(card->dev,
-					"could not configure MCLK state");
-				return ret;
-			}
+		ret = clk_prepare_enable(ctx->mclk);
+		if (ret < 0) {
+			dev_err(card->dev,
+				"could not configure MCLK state");
+			return ret;
 		}
 	} else {
 		/* Set codec sysclk source to its internal clock because codec PLL will
@@ -122,8 +120,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 			return ret;
 		}
 
-		if (ctx->mclk)
-			clk_disable_unprepare(ctx->mclk);
+		clk_disable_unprepare(ctx->mclk);
 	}
 
 	return 0;
@@ -258,11 +255,11 @@ static const struct dmi_system_id cht_rt5645_quirk_table[] = {
 
 static int cht_codec_init(struct snd_soc_pcm_runtime *runtime)
 {
-	int ret;
-	int jack_type;
-	struct snd_soc_codec *codec = runtime->codec;
 	struct snd_soc_card *card = runtime->card;
 	struct cht_mc_private *ctx = snd_soc_card_get_drvdata(runtime->card);
+	struct snd_soc_codec *codec = runtime->codec;
+	int jack_type;
+	int ret;
 
 	if ((cht_rt5645_quirk & CHT_RT5645_SSP2_AIF2) ||
 	    (cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) {
@@ -320,26 +317,26 @@ static int cht_codec_init(struct snd_soc_pcm_runtime *runtime)
 
 	rt5645_set_jack_detect(codec, &ctx->jack, &ctx->jack, &ctx->jack);
 
-	if (ctx->mclk) {
-		/*
-		 * The firmware might enable the clock at
-		 * boot (this information may or may not
-		 * be reflected in the enable clock register).
-		 * To change the rate we must disable the clock
-		 * first to cover these cases. Due to common
-		 * clock framework restrictions that do not allow
-		 * to disable a clock that has not been enabled,
-		 * we need to enable the clock first.
-		 */
-		ret = clk_prepare_enable(ctx->mclk);
-		if (!ret)
-			clk_disable_unprepare(ctx->mclk);
 
-		ret = clk_set_rate(ctx->mclk, CHT_PLAT_CLK_3_HZ);
+	/*
+	 * The firmware might enable the clock at
+	 * boot (this information may or may not
+	 * be reflected in the enable clock register).
+	 * To change the rate we must disable the clock
+	 * first to cover these cases. Due to common
+	 * clock framework restrictions that do not allow
+	 * to disable a clock that has not been enabled,
+	 * we need to enable the clock first.
+	 */
+	ret = clk_prepare_enable(ctx->mclk);
+	if (!ret)
+		clk_disable_unprepare(ctx->mclk);
+
+	ret = clk_set_rate(ctx->mclk, CHT_PLAT_CLK_3_HZ);
+
+	if (ret)
+		dev_err(runtime->dev, "unable to set MCLK rate\n");
 
-		if (ret)
-			dev_err(runtime->dev, "unable to set MCLK rate\n");
-	}
 	return ret;
 }
 
@@ -545,15 +542,15 @@ struct acpi_chan_package {   /* ACPICA seems to require 64 bit integers */
 
 static int snd_cht_mc_probe(struct platform_device *pdev)
 {
-	int ret_val = 0;
-	int i;
-	struct cht_mc_private *drv;
 	struct snd_soc_card *card = snd_soc_cards[0].soc_card;
 	struct sst_acpi_mach *mach;
+	struct cht_mc_private *drv;
 	const char *i2c_name = NULL;
-	int dai_index = 0;
 	bool found = false;
 	bool is_bytcr = false;
+	int dai_index = 0;
+	int ret_val = 0;
+	int i;
 
 	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_ATOMIC);
 	if (!drv)
@@ -590,7 +587,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 
 	/* fixup codec name based on HID */
 	i2c_name = sst_acpi_find_name_from_hid(mach->id);
-	if (i2c_name != NULL) {
+	if (i2c_name) {
 		snprintf(cht_rt5645_codec_name, sizeof(cht_rt5645_codec_name),
 			"%s%s", "i2c-", i2c_name);
 		cht_dailink[dai_index].codec_name = cht_rt5645_codec_name;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/6] ASoC: Intel: bytcht_da7213: cosmetic fixes
  2017-10-05 18:52 [PATCH 0/6] Baytrail-Cherrytrail updates Pierre-Louis Bossart
                   ` (3 preceding siblings ...)
  2017-10-05 18:52 ` [PATCH 4/6] ASoC: Intel: cht_bsw_rt5645: cosmetic fixes Pierre-Louis Bossart
@ 2017-10-05 18:52 ` Pierre-Louis Bossart
  2017-10-18 11:31   ` Applied "ASoC: Intel: bytcht_da7213: cosmetic fixes" to the asoc tree Mark Brown
  2017-10-05 18:52 ` [PATCH 6/6] ASoC: Intel: boards: use strcmp with constant literals Pierre-Louis Bossart
  5 siblings, 1 reply; 16+ messages in thread
From: Pierre-Louis Bossart @ 2017-10-05 18:52 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, vinod.koul, broonie, Pierre-Louis Bossart, Andy Shevchenko

Reorder variable names, change tests
No functional change

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/boards/bytcht_da7213.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcht_da7213.c b/sound/soc/intel/boards/bytcht_da7213.c
index 18873e23f404..24070bc06040 100644
--- a/sound/soc/intel/boards/bytcht_da7213.c
+++ b/sound/soc/intel/boards/bytcht_da7213.c
@@ -231,12 +231,12 @@ static char codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */
 
 static int bytcht_da7213_probe(struct platform_device *pdev)
 {
-	int ret_val = 0;
-	int i;
 	struct snd_soc_card *card;
 	struct sst_acpi_mach *mach;
 	const char *i2c_name = NULL;
 	int dai_index = 0;
+	int ret_val = 0;
+	int i;
 
 	mach = (&pdev->dev)->platform_data;
 	card = &bytcht_da7213_card;
@@ -253,7 +253,7 @@ static int bytcht_da7213_probe(struct platform_device *pdev)
 
 	/* fixup codec name based on HID */
 	i2c_name = sst_acpi_find_name_from_hid(mach->id);
-	if (i2c_name != NULL) {
+	if (i2c_name) {
 		snprintf(codec_name, sizeof(codec_name),
 			"%s%s", "i2c-", i2c_name);
 		dailink[dai_index].codec_name = codec_name;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/6] ASoC: Intel: boards: use strcmp with constant literals
  2017-10-05 18:52 [PATCH 0/6] Baytrail-Cherrytrail updates Pierre-Louis Bossart
                   ` (4 preceding siblings ...)
  2017-10-05 18:52 ` [PATCH 5/6] ASoC: Intel: bytcht_da7213: cosmetic fixes Pierre-Louis Bossart
@ 2017-10-05 18:52 ` Pierre-Louis Bossart
  2017-10-06  8:55   ` Takashi Iwai
  5 siblings, 1 reply; 16+ messages in thread
From: Pierre-Louis Bossart @ 2017-10-05 18:52 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, vinod.koul, broonie, Pierre-Louis Bossart, Andy Shevchenko

we used strncmp in all audio machine drivers, this is not needed
when comparing against constant literals.
General replace of strncmp(a, b, strlen(b)) to strcmp(a,b)

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/boards/bxt_da7219_max98357a.c   | 3 +--
 sound/soc/intel/boards/bytcht_es8316.c          | 3 +--
 sound/soc/intel/boards/bytcr_rt5640.c           | 6 ++----
 sound/soc/intel/boards/bytcr_rt5651.c           | 3 +--
 sound/soc/intel/boards/cht_bsw_max98090_ti.c    | 3 +--
 sound/soc/intel/boards/cht_bsw_rt5645.c         | 6 ++----
 sound/soc/intel/boards/cht_bsw_rt5672.c         | 3 +--
 sound/soc/intel/boards/skl_nau88l25_max98357a.c | 3 +--
 sound/soc/intel/boards/skl_nau88l25_ssm4567.c   | 3 +--
 9 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c
index ce35ec7884d1..dc47c3f0b621 100644
--- a/sound/soc/intel/boards/bxt_da7219_max98357a.c
+++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c
@@ -61,8 +61,7 @@ static inline struct snd_soc_dai *bxt_get_codec_dai(struct snd_soc_card *card)
 
 	list_for_each_entry(rtd, &card->rtd_list, list) {
 
-		if (!strncmp(rtd->codec_dai->name, BXT_DIALOG_CODEC_DAI,
-			     strlen(BXT_DIALOG_CODEC_DAI)))
+		if (!strcmp(rtd->codec_dai->name, BXT_DIALOG_CODEC_DAI))
 			return rtd->codec_dai;
 	}
 
diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
index 52635462dac6..9b42dc924cd3 100644
--- a/sound/soc/intel/boards/bytcht_es8316.c
+++ b/sound/soc/intel/boards/bytcht_es8316.c
@@ -44,8 +44,7 @@ static inline struct snd_soc_dai *get_codec_dai(struct snd_soc_card *card)
 	struct snd_soc_pcm_runtime *rtd;
 
 	list_for_each_entry(rtd, &card->rtd_list, list) {
-		if (!strncmp(rtd->codec_dai->name, CODEC_DAI1,
-			     strlen(CODEC_DAI1)))
+		if (!strcmp(rtd->codec_dai->name, CODEC_DAI1))
 			return rtd->codec_dai;
 	}
 	return NULL;
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index ffb3ca971df4..24c7f861f1e1 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -150,11 +150,9 @@ static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card)
 	struct snd_soc_pcm_runtime *rtd;
 
 	list_for_each_entry(rtd, &card->rtd_list, list) {
-		if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI1,
-			     strlen(BYT_CODEC_DAI1)))
+		if (!strcmp(rtd->codec_dai->name, BYT_CODEC_DAI1))
 			return rtd->codec_dai;
-		if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI2,
-				strlen(BYT_CODEC_DAI2)))
+		if (!strcmp(rtd->codec_dai->name, BYT_CODEC_DAI2))
 			return rtd->codec_dai;
 
 	}
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 1b97aa735747..0443d2e7e8a1 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -73,8 +73,7 @@ static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card)
 	struct snd_soc_pcm_runtime *rtd;
 
 	list_for_each_entry(rtd, &card->rtd_list, list) {
-		if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI1,
-			     strlen(BYT_CODEC_DAI1)))
+		if (!strcmp(rtd->codec_dai->name, BYT_CODEC_DAI1))
 			return rtd->codec_dai;
 	}
 	return NULL;
diff --git a/sound/soc/intel/boards/cht_bsw_max98090_ti.c b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
index 74c047a4b05c..98d58d389b04 100644
--- a/sound/soc/intel/boards/cht_bsw_max98090_ti.c
+++ b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
@@ -46,8 +46,7 @@ static inline struct snd_soc_dai *cht_get_codec_dai(struct snd_soc_card *card)
 	struct snd_soc_pcm_runtime *rtd;
 
 	list_for_each_entry(rtd, &card->rtd_list, list) {
-		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI,
-			     strlen(CHT_CODEC_DAI)))
+		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI))
 			return rtd->codec_dai;
 	}
 	return NULL;
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index 6ccb599c1082..e9a3121ecad7 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -75,11 +75,9 @@ static inline struct snd_soc_dai *cht_get_codec_dai(struct snd_soc_card *card)
 	struct snd_soc_pcm_runtime *rtd;
 
 	list_for_each_entry(rtd, &card->rtd_list, list) {
-		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI1,
-			     strlen(CHT_CODEC_DAI1)))
+		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI1))
 			return rtd->codec_dai;
-		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI2,
-			     strlen(CHT_CODEC_DAI2)))
+		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI2))
 			return rtd->codec_dai;
 	}
 	return NULL;
diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c
index f799b76fdedf..78798c600c16 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5672.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5672.c
@@ -55,8 +55,7 @@ static inline struct snd_soc_dai *cht_get_codec_dai(struct snd_soc_card *card)
 	struct snd_soc_pcm_runtime *rtd;
 
 	list_for_each_entry(rtd, &card->rtd_list, list) {
-		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI,
-			     strlen(CHT_CODEC_DAI)))
+		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI))
 			return rtd->codec_dai;
 	}
 	return NULL;
diff --git a/sound/soc/intel/boards/skl_nau88l25_max98357a.c b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
index 5ed0aa27b467..95c017f7315e 100644
--- a/sound/soc/intel/boards/skl_nau88l25_max98357a.c
+++ b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
@@ -60,8 +60,7 @@ static inline struct snd_soc_dai *skl_get_codec_dai(struct snd_soc_card *card)
 
 	list_for_each_entry(rtd, &card->rtd_list, list) {
 
-		if (!strncmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI,
-			     strlen(SKL_NUVOTON_CODEC_DAI)))
+		if (!strcmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI))
 			return rtd->codec_dai;
 	}
 
diff --git a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
index 01b8b140bb08..57f1e5b8408f 100644
--- a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
+++ b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
@@ -63,8 +63,7 @@ static inline struct snd_soc_dai *skl_get_codec_dai(struct snd_soc_card *card)
 
 	list_for_each_entry(rtd, &card->rtd_list, list) {
 
-		if (!strncmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI,
-			     strlen(SKL_NUVOTON_CODEC_DAI)))
+		if (!strcmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI))
 			return rtd->codec_dai;
 	}
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] ASoC: Intel: boards: use strcmp with constant literals
  2017-10-05 18:52 ` [PATCH 6/6] ASoC: Intel: boards: use strcmp with constant literals Pierre-Louis Bossart
@ 2017-10-06  8:55   ` Takashi Iwai
  2017-10-06 11:50     ` Andy Shevchenko
  2017-10-09  9:34     ` Vinod Koul
  0 siblings, 2 replies; 16+ messages in thread
From: Takashi Iwai @ 2017-10-06  8:55 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: vinod.koul, alsa-devel, broonie, Andy Shevchenko

On Thu, 05 Oct 2017 20:52:43 +0200,
Pierre-Louis Bossart wrote:
> 
> we used strncmp in all audio machine drivers, this is not needed
> when comparing against constant literals.
> General replace of strncmp(a, b, strlen(b)) to strcmp(a,b)
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  sound/soc/intel/boards/bxt_da7219_max98357a.c   | 3 +--
>  sound/soc/intel/boards/bytcht_es8316.c          | 3 +--
>  sound/soc/intel/boards/bytcr_rt5640.c           | 6 ++----
>  sound/soc/intel/boards/bytcr_rt5651.c           | 3 +--
>  sound/soc/intel/boards/cht_bsw_max98090_ti.c    | 3 +--
>  sound/soc/intel/boards/cht_bsw_rt5645.c         | 6 ++----
>  sound/soc/intel/boards/cht_bsw_rt5672.c         | 3 +--
>  sound/soc/intel/boards/skl_nau88l25_max98357a.c | 3 +--
>  sound/soc/intel/boards/skl_nau88l25_ssm4567.c   | 3 +--
>  9 files changed, 11 insertions(+), 22 deletions(-)

While we're at it: since all these do mostly same: wouldn't it make
more sense to create a helper to scan over the rtd_list?

static inline snd_soc_intel_get_codec_dai(struct snd_soc_card *card,
					  const char *dai_name)
{
	struct snd_soc_pcm_runtime *rtd;

	list_for_each_entry(rtd, &card->rtd_list, list) {
		if (!strcmp(rtd->codec_dai->name, dai_name))
			return rtd->codec_dai;
	}
	
	return NULL;
}

Some drivers require to check two names, but it can call this function
twice.


thanks,

Takashi

> diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c
> index ce35ec7884d1..dc47c3f0b621 100644
> --- a/sound/soc/intel/boards/bxt_da7219_max98357a.c
> +++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c
> @@ -61,8 +61,7 @@ static inline struct snd_soc_dai *bxt_get_codec_dai(struct snd_soc_card *card)
>  
>  	list_for_each_entry(rtd, &card->rtd_list, list) {
>  
> -		if (!strncmp(rtd->codec_dai->name, BXT_DIALOG_CODEC_DAI,
> -			     strlen(BXT_DIALOG_CODEC_DAI)))
> +		if (!strcmp(rtd->codec_dai->name, BXT_DIALOG_CODEC_DAI))
>  			return rtd->codec_dai;
>  	}
>  
> diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
> index 52635462dac6..9b42dc924cd3 100644
> --- a/sound/soc/intel/boards/bytcht_es8316.c
> +++ b/sound/soc/intel/boards/bytcht_es8316.c
> @@ -44,8 +44,7 @@ static inline struct snd_soc_dai *get_codec_dai(struct snd_soc_card *card)
>  	struct snd_soc_pcm_runtime *rtd;
>  
>  	list_for_each_entry(rtd, &card->rtd_list, list) {
> -		if (!strncmp(rtd->codec_dai->name, CODEC_DAI1,
> -			     strlen(CODEC_DAI1)))
> +		if (!strcmp(rtd->codec_dai->name, CODEC_DAI1))
>  			return rtd->codec_dai;
>  	}
>  	return NULL;
> diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
> index ffb3ca971df4..24c7f861f1e1 100644
> --- a/sound/soc/intel/boards/bytcr_rt5640.c
> +++ b/sound/soc/intel/boards/bytcr_rt5640.c
> @@ -150,11 +150,9 @@ static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card)
>  	struct snd_soc_pcm_runtime *rtd;
>  
>  	list_for_each_entry(rtd, &card->rtd_list, list) {
> -		if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI1,
> -			     strlen(BYT_CODEC_DAI1)))
> +		if (!strcmp(rtd->codec_dai->name, BYT_CODEC_DAI1))
>  			return rtd->codec_dai;
> -		if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI2,
> -				strlen(BYT_CODEC_DAI2)))
> +		if (!strcmp(rtd->codec_dai->name, BYT_CODEC_DAI2))
>  			return rtd->codec_dai;
>  
>  	}
> diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
> index 1b97aa735747..0443d2e7e8a1 100644
> --- a/sound/soc/intel/boards/bytcr_rt5651.c
> +++ b/sound/soc/intel/boards/bytcr_rt5651.c
> @@ -73,8 +73,7 @@ static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card)
>  	struct snd_soc_pcm_runtime *rtd;
>  
>  	list_for_each_entry(rtd, &card->rtd_list, list) {
> -		if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI1,
> -			     strlen(BYT_CODEC_DAI1)))
> +		if (!strcmp(rtd->codec_dai->name, BYT_CODEC_DAI1))
>  			return rtd->codec_dai;
>  	}
>  	return NULL;
> diff --git a/sound/soc/intel/boards/cht_bsw_max98090_ti.c b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
> index 74c047a4b05c..98d58d389b04 100644
> --- a/sound/soc/intel/boards/cht_bsw_max98090_ti.c
> +++ b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
> @@ -46,8 +46,7 @@ static inline struct snd_soc_dai *cht_get_codec_dai(struct snd_soc_card *card)
>  	struct snd_soc_pcm_runtime *rtd;
>  
>  	list_for_each_entry(rtd, &card->rtd_list, list) {
> -		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI,
> -			     strlen(CHT_CODEC_DAI)))
> +		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI))
>  			return rtd->codec_dai;
>  	}
>  	return NULL;
> diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
> index 6ccb599c1082..e9a3121ecad7 100644
> --- a/sound/soc/intel/boards/cht_bsw_rt5645.c
> +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
> @@ -75,11 +75,9 @@ static inline struct snd_soc_dai *cht_get_codec_dai(struct snd_soc_card *card)
>  	struct snd_soc_pcm_runtime *rtd;
>  
>  	list_for_each_entry(rtd, &card->rtd_list, list) {
> -		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI1,
> -			     strlen(CHT_CODEC_DAI1)))
> +		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI1))
>  			return rtd->codec_dai;
> -		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI2,
> -			     strlen(CHT_CODEC_DAI2)))
> +		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI2))
>  			return rtd->codec_dai;
>  	}
>  	return NULL;
> diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c
> index f799b76fdedf..78798c600c16 100644
> --- a/sound/soc/intel/boards/cht_bsw_rt5672.c
> +++ b/sound/soc/intel/boards/cht_bsw_rt5672.c
> @@ -55,8 +55,7 @@ static inline struct snd_soc_dai *cht_get_codec_dai(struct snd_soc_card *card)
>  	struct snd_soc_pcm_runtime *rtd;
>  
>  	list_for_each_entry(rtd, &card->rtd_list, list) {
> -		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI,
> -			     strlen(CHT_CODEC_DAI)))
> +		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI))
>  			return rtd->codec_dai;
>  	}
>  	return NULL;
> diff --git a/sound/soc/intel/boards/skl_nau88l25_max98357a.c b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> index 5ed0aa27b467..95c017f7315e 100644
> --- a/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> +++ b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> @@ -60,8 +60,7 @@ static inline struct snd_soc_dai *skl_get_codec_dai(struct snd_soc_card *card)
>  
>  	list_for_each_entry(rtd, &card->rtd_list, list) {
>  
> -		if (!strncmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI,
> -			     strlen(SKL_NUVOTON_CODEC_DAI)))
> +		if (!strcmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI))
>  			return rtd->codec_dai;
>  	}
>  
> diff --git a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
> index 01b8b140bb08..57f1e5b8408f 100644
> --- a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
> +++ b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
> @@ -63,8 +63,7 @@ static inline struct snd_soc_dai *skl_get_codec_dai(struct snd_soc_card *card)
>  
>  	list_for_each_entry(rtd, &card->rtd_list, list) {
>  
> -		if (!strncmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI,
> -			     strlen(SKL_NUVOTON_CODEC_DAI)))
> +		if (!strcmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI))
>  			return rtd->codec_dai;
>  	}
>  
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] ASoC: Intel: boards: use strcmp with constant literals
  2017-10-06  8:55   ` Takashi Iwai
@ 2017-10-06 11:50     ` Andy Shevchenko
  2017-10-06 14:28       ` Pierre-Louis Bossart
  2017-10-09  9:34     ` Vinod Koul
  1 sibling, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2017-10-06 11:50 UTC (permalink / raw)
  To: Takashi Iwai, Pierre-Louis Bossart; +Cc: vinod.koul, alsa-devel, broonie

On Fri, 2017-10-06 at 10:55 +0200, Takashi Iwai wrote:
> On Thu, 05 Oct 2017 20:52:43 +0200,
> Pierre-Louis Bossart wrote:
> > 
> > we used strncmp in all audio machine drivers, this is not needed
> > when comparing against constant literals.
> > General replace of strncmp(a, b, strlen(b)) to strcmp(a,b)
> > 
> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.inte
> > l.com>
> > ---
> >  sound/soc/intel/boards/bxt_da7219_max98357a.c   | 3 +--
> >  sound/soc/intel/boards/bytcht_es8316.c          | 3 +--
> >  sound/soc/intel/boards/bytcr_rt5640.c           | 6 ++----
> >  sound/soc/intel/boards/bytcr_rt5651.c           | 3 +--
> >  sound/soc/intel/boards/cht_bsw_max98090_ti.c    | 3 +--
> >  sound/soc/intel/boards/cht_bsw_rt5645.c         | 6 ++----
> >  sound/soc/intel/boards/cht_bsw_rt5672.c         | 3 +--
> >  sound/soc/intel/boards/skl_nau88l25_max98357a.c | 3 +--
> >  sound/soc/intel/boards/skl_nau88l25_ssm4567.c   | 3 +--
> >  9 files changed, 11 insertions(+), 22 deletions(-)
> 
> While we're at it: since all these do mostly same: wouldn't it make
> more sense to create a helper to scan over the rtd_list?
> 
> static inline snd_soc_intel_get_codec_dai(struct snd_soc_card *card,
> 					  const char *dai_name)
> {
> 	struct snd_soc_pcm_runtime *rtd;
> 
> 	list_for_each_entry(rtd, &card->rtd_list, list) {
> 		if (!strcmp(rtd->codec_dai->name, dai_name))
> 			return rtd->codec_dai;
> 	}
> 	
> 	return NULL;
> }

I like this!

> Some drivers require to check two names, but it can call this function
> twice.

It looks like not hot path, so, 2 passes instead of one might be not a
big deal.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] ASoC: Intel: boards: use strcmp with constant literals
  2017-10-06 11:50     ` Andy Shevchenko
@ 2017-10-06 14:28       ` Pierre-Louis Bossart
  2017-10-06 15:24         ` Pierre-Louis Bossart
  0 siblings, 1 reply; 16+ messages in thread
From: Pierre-Louis Bossart @ 2017-10-06 14:28 UTC (permalink / raw)
  To: Andy Shevchenko, Takashi Iwai; +Cc: vinod.koul, alsa-devel, broonie

On 10/6/17 6:50 AM, Andy Shevchenko wrote:
> On Fri, 2017-10-06 at 10:55 +0200, Takashi Iwai wrote:
>> On Thu, 05 Oct 2017 20:52:43 +0200,
>> Pierre-Louis Bossart wrote:
>>>
>>> we used strncmp in all audio machine drivers, this is not needed
>>> when comparing against constant literals.
>>> General replace of strncmp(a, b, strlen(b)) to strcmp(a,b)
>>>
>>> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.inte
>>> l.com>
>>> ---
>>>   sound/soc/intel/boards/bxt_da7219_max98357a.c   | 3 +--
>>>   sound/soc/intel/boards/bytcht_es8316.c          | 3 +--
>>>   sound/soc/intel/boards/bytcr_rt5640.c           | 6 ++----
>>>   sound/soc/intel/boards/bytcr_rt5651.c           | 3 +--
>>>   sound/soc/intel/boards/cht_bsw_max98090_ti.c    | 3 +--
>>>   sound/soc/intel/boards/cht_bsw_rt5645.c         | 6 ++----
>>>   sound/soc/intel/boards/cht_bsw_rt5672.c         | 3 +--
>>>   sound/soc/intel/boards/skl_nau88l25_max98357a.c | 3 +--
>>>   sound/soc/intel/boards/skl_nau88l25_ssm4567.c   | 3 +--
>>>   9 files changed, 11 insertions(+), 22 deletions(-)
>>
>> While we're at it: since all these do mostly same: wouldn't it make
>> more sense to create a helper to scan over the rtd_list?
>>
>> static inline snd_soc_intel_get_codec_dai(struct snd_soc_card *card,
>> 					  const char *dai_name)
>> {
>> 	struct snd_soc_pcm_runtime *rtd;
>>
>> 	list_for_each_entry(rtd, &card->rtd_list, list) {
>> 		if (!strcmp(rtd->codec_dai->name, dai_name))
>> 			return rtd->codec_dai;
>> 	}
>> 	
>> 	return NULL;
>> }
> 
> I like this!
> 
>> Some drivers require to check two names, but it can call this function
>> twice.
> 
> It looks like not hot path, so, 2 passes instead of one might be not a
> big deal.

Yes, that'd be a good way to simplify further. I'll resend the series, 
thanks for the suggestion Takashi.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] ASoC: Intel: boards: use strcmp with constant literals
  2017-10-06 14:28       ` Pierre-Louis Bossart
@ 2017-10-06 15:24         ` Pierre-Louis Bossart
  2017-10-06 15:48           ` Takashi Iwai
  0 siblings, 1 reply; 16+ messages in thread
From: Pierre-Louis Bossart @ 2017-10-06 15:24 UTC (permalink / raw)
  To: Andy Shevchenko, Takashi Iwai; +Cc: vinod.koul, alsa-devel, broonie



On 10/06/2017 09:28 AM, Pierre-Louis Bossart wrote:
> On 10/6/17 6:50 AM, Andy Shevchenko wrote:
>> On Fri, 2017-10-06 at 10:55 +0200, Takashi Iwai wrote:
>>> On Thu, 05 Oct 2017 20:52:43 +0200,
>>> Pierre-Louis Bossart wrote:
>>>>
>>>> we used strncmp in all audio machine drivers, this is not needed
>>>> when comparing against constant literals.
>>>> General replace of strncmp(a, b, strlen(b)) to strcmp(a,b)
>>>>
>>>> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.inte
>>>> l.com>
>>>> ---
>>>>   sound/soc/intel/boards/bxt_da7219_max98357a.c   | 3 +--
>>>>   sound/soc/intel/boards/bytcht_es8316.c          | 3 +--
>>>>   sound/soc/intel/boards/bytcr_rt5640.c           | 6 ++----
>>>>   sound/soc/intel/boards/bytcr_rt5651.c           | 3 +--
>>>>   sound/soc/intel/boards/cht_bsw_max98090_ti.c    | 3 +--
>>>>   sound/soc/intel/boards/cht_bsw_rt5645.c         | 6 ++----
>>>>   sound/soc/intel/boards/cht_bsw_rt5672.c         | 3 +--
>>>>   sound/soc/intel/boards/skl_nau88l25_max98357a.c | 3 +--
>>>>   sound/soc/intel/boards/skl_nau88l25_ssm4567.c   | 3 +--
>>>>   9 files changed, 11 insertions(+), 22 deletions(-)
>>>
>>> While we're at it: since all these do mostly same: wouldn't it make
>>> more sense to create a helper to scan over the rtd_list?
>>>
>>> static inline snd_soc_intel_get_codec_dai(struct snd_soc_card *card,
>>>                       const char *dai_name)
>>> {
>>>     struct snd_soc_pcm_runtime *rtd;
>>>
>>>     list_for_each_entry(rtd, &card->rtd_list, list) {
>>>         if (!strcmp(rtd->codec_dai->name, dai_name))
>>>             return rtd->codec_dai;
>>>     }
>>>
>>>     return NULL;
>>> }
>>
>> I like this!
>>
>>> Some drivers require to check two names, but it can call this function
>>> twice.
>>
>> It looks like not hot path, so, 2 passes instead of one might be not a
>> big deal.
>
> Yes, that'd be a good way to simplify further. I'll resend the series, 
> thanks for the suggestion Takashi.
The code is not Intel-specific though, should it be moved to soc.h ? e.g.

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 11ca867033bd..68f829d32c76 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1736,6 +1736,19 @@ int snd_soc_register_dai(struct snd_soc_component 
*component,
  struct snd_soc_dai *snd_soc_find_dai(
         const struct snd_soc_dai_link_component *dlc);

+static inline snd_soc_card_get_codec_dai(struct snd_soc_card *card,
+                                        const char *dai_name)
+{
+       struct snd_soc_pcm_runtime *rtd;
+
+       list_for_each_entry(rtd, &card->rtd_list, list) {
+               if (!strcmp(rtd->codec_dai->name, dai_name))
+                       return rtd->codec_dai;
+       }
+
+       return NULL;
+}
+
  #include <sound/soc-dai.h>

  #ifdef CONFIG_DEBUG_FS




_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] ASoC: Intel: boards: use strcmp with constant literals
  2017-10-06 15:24         ` Pierre-Louis Bossart
@ 2017-10-06 15:48           ` Takashi Iwai
  0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2017-10-06 15:48 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: vinod.koul, alsa-devel, Andy Shevchenko, broonie

On Fri, 06 Oct 2017 17:24:22 +0200,
Pierre-Louis Bossart wrote:
> 
> 
> 
> On 10/06/2017 09:28 AM, Pierre-Louis Bossart wrote:
> > On 10/6/17 6:50 AM, Andy Shevchenko wrote:
> >> On Fri, 2017-10-06 at 10:55 +0200, Takashi Iwai wrote:
> >>> On Thu, 05 Oct 2017 20:52:43 +0200,
> >>> Pierre-Louis Bossart wrote:
> >>>>
> >>>> we used strncmp in all audio machine drivers, this is not needed
> >>>> when comparing against constant literals.
> >>>> General replace of strncmp(a, b, strlen(b)) to strcmp(a,b)
> >>>>
> >>>> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.inte
> >>>> l.com>
> >>>> ---
> >>>>   sound/soc/intel/boards/bxt_da7219_max98357a.c   | 3 +--
> >>>>   sound/soc/intel/boards/bytcht_es8316.c          | 3 +--
> >>>>   sound/soc/intel/boards/bytcr_rt5640.c           | 6 ++----
> >>>>   sound/soc/intel/boards/bytcr_rt5651.c           | 3 +--
> >>>>   sound/soc/intel/boards/cht_bsw_max98090_ti.c    | 3 +--
> >>>>   sound/soc/intel/boards/cht_bsw_rt5645.c         | 6 ++----
> >>>>   sound/soc/intel/boards/cht_bsw_rt5672.c         | 3 +--
> >>>>   sound/soc/intel/boards/skl_nau88l25_max98357a.c | 3 +--
> >>>>   sound/soc/intel/boards/skl_nau88l25_ssm4567.c   | 3 +--
> >>>>   9 files changed, 11 insertions(+), 22 deletions(-)
> >>>
> >>> While we're at it: since all these do mostly same: wouldn't it make
> >>> more sense to create a helper to scan over the rtd_list?
> >>>
> >>> static inline snd_soc_intel_get_codec_dai(struct snd_soc_card *card,
> >>>                       const char *dai_name)
> >>> {
> >>>     struct snd_soc_pcm_runtime *rtd;
> >>>
> >>>     list_for_each_entry(rtd, &card->rtd_list, list) {
> >>>         if (!strcmp(rtd->codec_dai->name, dai_name))
> >>>             return rtd->codec_dai;
> >>>     }
> >>>
> >>>     return NULL;
> >>> }
> >>
> >> I like this!
> >>
> >>> Some drivers require to check two names, but it can call this function
> >>> twice.
> >>
> >> It looks like not hot path, so, 2 passes instead of one might be not a
> >> big deal.
> >
> > Yes, that'd be a good way to simplify further. I'll resend the
> > series, thanks for the suggestion Takashi.
> The code is not Intel-specific though, should it be moved to soc.h ? e.g.
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 11ca867033bd..68f829d32c76 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -1736,6 +1736,19 @@ int snd_soc_register_dai(struct
> snd_soc_component *component,
>  struct snd_soc_dai *snd_soc_find_dai(
>         const struct snd_soc_dai_link_component *dlc);
> 
> +static inline snd_soc_card_get_codec_dai(struct snd_soc_card *card,
> +                                        const char *dai_name)
> +{
> +       struct snd_soc_pcm_runtime *rtd;
> +
> +       list_for_each_entry(rtd, &card->rtd_list, list) {
> +               if (!strcmp(rtd->codec_dai->name, dai_name))
> +                       return rtd->codec_dai;
> +       }
> +
> +       return NULL;
> +}

Currently only Intel drivers use this, so I guess we need no hustle to
lift up for now.  But it's an inline function, so it's basically
harmless, so I'd leave the decision to Mark.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] ASoC: Intel: boards: use strcmp with constant literals
  2017-10-06  8:55   ` Takashi Iwai
  2017-10-06 11:50     ` Andy Shevchenko
@ 2017-10-09  9:34     ` Vinod Koul
  1 sibling, 0 replies; 16+ messages in thread
From: Vinod Koul @ 2017-10-09  9:34 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, broonie, Pierre-Louis Bossart, Andy Shevchenko

On Fri, Oct 06, 2017 at 10:55:33AM +0200, Takashi Iwai wrote:
> On Thu, 05 Oct 2017 20:52:43 +0200,
> Pierre-Louis Bossart wrote:
> > 
> > we used strncmp in all audio machine drivers, this is not needed
> > when comparing against constant literals.
> > General replace of strncmp(a, b, strlen(b)) to strcmp(a,b)
> > 
> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > ---
> >  sound/soc/intel/boards/bxt_da7219_max98357a.c   | 3 +--
> >  sound/soc/intel/boards/bytcht_es8316.c          | 3 +--
> >  sound/soc/intel/boards/bytcr_rt5640.c           | 6 ++----
> >  sound/soc/intel/boards/bytcr_rt5651.c           | 3 +--
> >  sound/soc/intel/boards/cht_bsw_max98090_ti.c    | 3 +--
> >  sound/soc/intel/boards/cht_bsw_rt5645.c         | 6 ++----
> >  sound/soc/intel/boards/cht_bsw_rt5672.c         | 3 +--
> >  sound/soc/intel/boards/skl_nau88l25_max98357a.c | 3 +--
> >  sound/soc/intel/boards/skl_nau88l25_ssm4567.c   | 3 +--
> >  9 files changed, 11 insertions(+), 22 deletions(-)
> 
> While we're at it: since all these do mostly same: wouldn't it make
> more sense to create a helper to scan over the rtd_list?
> 
> static inline snd_soc_intel_get_codec_dai(struct snd_soc_card *card,
> 					  const char *dai_name)
> {
> 	struct snd_soc_pcm_runtime *rtd;
> 
> 	list_for_each_entry(rtd, &card->rtd_list, list) {
> 		if (!strcmp(rtd->codec_dai->name, dai_name))
> 			return rtd->codec_dai;
> 	}
> 	
> 	return NULL;
> }
> 
> Some drivers require to check two names, but it can call this function
> twice.

yeah this was in my todo :( would be gald if its done now :)

> > diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c
> > index ce35ec7884d1..dc47c3f0b621 100644
> > --- a/sound/soc/intel/boards/bxt_da7219_max98357a.c
> > +++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c
> > @@ -61,8 +61,7 @@ static inline struct snd_soc_dai *bxt_get_codec_dai(struct snd_soc_card *card)
> >  
> >  	list_for_each_entry(rtd, &card->rtd_list, list) {
> >  
> > -		if (!strncmp(rtd->codec_dai->name, BXT_DIALOG_CODEC_DAI,
> > -			     strlen(BXT_DIALOG_CODEC_DAI)))
> > +		if (!strcmp(rtd->codec_dai->name, BXT_DIALOG_CODEC_DAI))
> >  			return rtd->codec_dai;
> >  	}
> >  
> > diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
> > index 52635462dac6..9b42dc924cd3 100644
> > --- a/sound/soc/intel/boards/bytcht_es8316.c
> > +++ b/sound/soc/intel/boards/bytcht_es8316.c
> > @@ -44,8 +44,7 @@ static inline struct snd_soc_dai *get_codec_dai(struct snd_soc_card *card)
> >  	struct snd_soc_pcm_runtime *rtd;
> >  
> >  	list_for_each_entry(rtd, &card->rtd_list, list) {
> > -		if (!strncmp(rtd->codec_dai->name, CODEC_DAI1,
> > -			     strlen(CODEC_DAI1)))
> > +		if (!strcmp(rtd->codec_dai->name, CODEC_DAI1))
> >  			return rtd->codec_dai;
> >  	}
> >  	return NULL;
> > diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
> > index ffb3ca971df4..24c7f861f1e1 100644
> > --- a/sound/soc/intel/boards/bytcr_rt5640.c
> > +++ b/sound/soc/intel/boards/bytcr_rt5640.c
> > @@ -150,11 +150,9 @@ static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card)
> >  	struct snd_soc_pcm_runtime *rtd;
> >  
> >  	list_for_each_entry(rtd, &card->rtd_list, list) {
> > -		if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI1,
> > -			     strlen(BYT_CODEC_DAI1)))
> > +		if (!strcmp(rtd->codec_dai->name, BYT_CODEC_DAI1))
> >  			return rtd->codec_dai;
> > -		if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI2,
> > -				strlen(BYT_CODEC_DAI2)))
> > +		if (!strcmp(rtd->codec_dai->name, BYT_CODEC_DAI2))
> >  			return rtd->codec_dai;
> >  
> >  	}
> > diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
> > index 1b97aa735747..0443d2e7e8a1 100644
> > --- a/sound/soc/intel/boards/bytcr_rt5651.c
> > +++ b/sound/soc/intel/boards/bytcr_rt5651.c
> > @@ -73,8 +73,7 @@ static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card)
> >  	struct snd_soc_pcm_runtime *rtd;
> >  
> >  	list_for_each_entry(rtd, &card->rtd_list, list) {
> > -		if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI1,
> > -			     strlen(BYT_CODEC_DAI1)))
> > +		if (!strcmp(rtd->codec_dai->name, BYT_CODEC_DAI1))
> >  			return rtd->codec_dai;
> >  	}
> >  	return NULL;
> > diff --git a/sound/soc/intel/boards/cht_bsw_max98090_ti.c b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
> > index 74c047a4b05c..98d58d389b04 100644
> > --- a/sound/soc/intel/boards/cht_bsw_max98090_ti.c
> > +++ b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
> > @@ -46,8 +46,7 @@ static inline struct snd_soc_dai *cht_get_codec_dai(struct snd_soc_card *card)
> >  	struct snd_soc_pcm_runtime *rtd;
> >  
> >  	list_for_each_entry(rtd, &card->rtd_list, list) {
> > -		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI,
> > -			     strlen(CHT_CODEC_DAI)))
> > +		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI))
> >  			return rtd->codec_dai;
> >  	}
> >  	return NULL;
> > diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
> > index 6ccb599c1082..e9a3121ecad7 100644
> > --- a/sound/soc/intel/boards/cht_bsw_rt5645.c
> > +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
> > @@ -75,11 +75,9 @@ static inline struct snd_soc_dai *cht_get_codec_dai(struct snd_soc_card *card)
> >  	struct snd_soc_pcm_runtime *rtd;
> >  
> >  	list_for_each_entry(rtd, &card->rtd_list, list) {
> > -		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI1,
> > -			     strlen(CHT_CODEC_DAI1)))
> > +		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI1))
> >  			return rtd->codec_dai;
> > -		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI2,
> > -			     strlen(CHT_CODEC_DAI2)))
> > +		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI2))
> >  			return rtd->codec_dai;
> >  	}
> >  	return NULL;
> > diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c
> > index f799b76fdedf..78798c600c16 100644
> > --- a/sound/soc/intel/boards/cht_bsw_rt5672.c
> > +++ b/sound/soc/intel/boards/cht_bsw_rt5672.c
> > @@ -55,8 +55,7 @@ static inline struct snd_soc_dai *cht_get_codec_dai(struct snd_soc_card *card)
> >  	struct snd_soc_pcm_runtime *rtd;
> >  
> >  	list_for_each_entry(rtd, &card->rtd_list, list) {
> > -		if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI,
> > -			     strlen(CHT_CODEC_DAI)))
> > +		if (!strcmp(rtd->codec_dai->name, CHT_CODEC_DAI))
> >  			return rtd->codec_dai;
> >  	}
> >  	return NULL;
> > diff --git a/sound/soc/intel/boards/skl_nau88l25_max98357a.c b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> > index 5ed0aa27b467..95c017f7315e 100644
> > --- a/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> > +++ b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> > @@ -60,8 +60,7 @@ static inline struct snd_soc_dai *skl_get_codec_dai(struct snd_soc_card *card)
> >  
> >  	list_for_each_entry(rtd, &card->rtd_list, list) {
> >  
> > -		if (!strncmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI,
> > -			     strlen(SKL_NUVOTON_CODEC_DAI)))
> > +		if (!strcmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI))
> >  			return rtd->codec_dai;
> >  	}
> >  
> > diff --git a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
> > index 01b8b140bb08..57f1e5b8408f 100644
> > --- a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
> > +++ b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
> > @@ -63,8 +63,7 @@ static inline struct snd_soc_dai *skl_get_codec_dai(struct snd_soc_card *card)
> >  
> >  	list_for_each_entry(rtd, &card->rtd_list, list) {
> >  
> > -		if (!strncmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI,
> > -			     strlen(SKL_NUVOTON_CODEC_DAI)))
> > +		if (!strcmp(rtd->codec_dai->name, SKL_NUVOTON_CODEC_DAI))
> >  			return rtd->codec_dai;
> >  	}
> >  
> > -- 
> > 2.11.0
> > 

-- 
~Vinod

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Applied "ASoC: Intel: bytcht_da7213: cosmetic fixes" to the asoc tree
  2017-10-05 18:52 ` [PATCH 5/6] ASoC: Intel: bytcht_da7213: cosmetic fixes Pierre-Louis Bossart
@ 2017-10-18 11:31   ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2017-10-18 11:31 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: alsa-devel, tiwai, Liam Girdwood, vinod.koul, broonie, Andy Shevchenko

The patch

   ASoC: Intel: bytcht_da7213: cosmetic fixes

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 22a317a0505d11db3ab08f4e4b25d3064228af02 Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Date: Thu, 12 Oct 2017 18:38:00 -0500
Subject: [PATCH] ASoC: Intel: bytcht_da7213: cosmetic fixes

Reorder variable names, change tests
No functional change

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Acked-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcht_da7213.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcht_da7213.c b/sound/soc/intel/boards/bytcht_da7213.c
index 18873e23f404..24070bc06040 100644
--- a/sound/soc/intel/boards/bytcht_da7213.c
+++ b/sound/soc/intel/boards/bytcht_da7213.c
@@ -231,12 +231,12 @@ static char codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */
 
 static int bytcht_da7213_probe(struct platform_device *pdev)
 {
-	int ret_val = 0;
-	int i;
 	struct snd_soc_card *card;
 	struct sst_acpi_mach *mach;
 	const char *i2c_name = NULL;
 	int dai_index = 0;
+	int ret_val = 0;
+	int i;
 
 	mach = (&pdev->dev)->platform_data;
 	card = &bytcht_da7213_card;
@@ -253,7 +253,7 @@ static int bytcht_da7213_probe(struct platform_device *pdev)
 
 	/* fixup codec name based on HID */
 	i2c_name = sst_acpi_find_name_from_hid(mach->id);
-	if (i2c_name != NULL) {
+	if (i2c_name) {
 		snprintf(codec_name, sizeof(codec_name),
 			"%s%s", "i2c-", i2c_name);
 		dailink[dai_index].codec_name = codec_name;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Applied "ASoC: Intel: cht_bsw_rt5645: cosmetic fixes" to the asoc tree
  2017-10-05 18:52 ` [PATCH 4/6] ASoC: Intel: cht_bsw_rt5645: cosmetic fixes Pierre-Louis Bossart
@ 2017-10-18 11:31   ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2017-10-18 11:31 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: alsa-devel, tiwai, Liam Girdwood, vinod.koul, broonie, Andy Shevchenko

The patch

   ASoC: Intel: cht_bsw_rt5645: cosmetic fixes

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 6cdf01a5addfd3a2b2918fccc76535f20eb2cda9 Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Date: Thu, 12 Oct 2017 18:37:59 -0500
Subject: [PATCH] ASoC: Intel: cht_bsw_rt5645: cosmetic fixes

Reorder variable names, change MCLK test, change quirks
No functional change

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Acked-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/cht_bsw_rt5645.c | 73 ++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index d553e2b67c92..6ccb599c1082 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -21,13 +21,13 @@
  */
 
 #include <linux/module.h>
-#include <linux/acpi.h>
 #include <linux/platform_device.h>
+#include <linux/acpi.h>
+#include <linux/clk.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
 #include <asm/cpu_device_id.h>
 #include <asm/platform_sst_audio.h>
-#include <linux/clk.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
@@ -53,7 +53,7 @@ struct cht_mc_private {
 	struct clk *mclk;
 };
 
-#define CHT_RT5645_MAP(quirk)	((quirk) & 0xff)
+#define CHT_RT5645_MAP(quirk)	((quirk) & GENMASK(7, 0))
 #define CHT_RT5645_SSP2_AIF2     BIT(16) /* default is using AIF1  */
 #define CHT_RT5645_SSP0_AIF1     BIT(17)
 #define CHT_RT5645_SSP0_AIF2     BIT(18)
@@ -101,13 +101,11 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 	}
 
 	if (SND_SOC_DAPM_EVENT_ON(event)) {
-		if (ctx->mclk) {
-			ret = clk_prepare_enable(ctx->mclk);
-			if (ret < 0) {
-				dev_err(card->dev,
-					"could not configure MCLK state");
-				return ret;
-			}
+		ret = clk_prepare_enable(ctx->mclk);
+		if (ret < 0) {
+			dev_err(card->dev,
+				"could not configure MCLK state");
+			return ret;
 		}
 	} else {
 		/* Set codec sysclk source to its internal clock because codec PLL will
@@ -122,8 +120,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 			return ret;
 		}
 
-		if (ctx->mclk)
-			clk_disable_unprepare(ctx->mclk);
+		clk_disable_unprepare(ctx->mclk);
 	}
 
 	return 0;
@@ -258,11 +255,11 @@ static const struct dmi_system_id cht_rt5645_quirk_table[] = {
 
 static int cht_codec_init(struct snd_soc_pcm_runtime *runtime)
 {
-	int ret;
-	int jack_type;
-	struct snd_soc_codec *codec = runtime->codec;
 	struct snd_soc_card *card = runtime->card;
 	struct cht_mc_private *ctx = snd_soc_card_get_drvdata(runtime->card);
+	struct snd_soc_codec *codec = runtime->codec;
+	int jack_type;
+	int ret;
 
 	if ((cht_rt5645_quirk & CHT_RT5645_SSP2_AIF2) ||
 	    (cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)) {
@@ -320,26 +317,26 @@ static int cht_codec_init(struct snd_soc_pcm_runtime *runtime)
 
 	rt5645_set_jack_detect(codec, &ctx->jack, &ctx->jack, &ctx->jack);
 
-	if (ctx->mclk) {
-		/*
-		 * The firmware might enable the clock at
-		 * boot (this information may or may not
-		 * be reflected in the enable clock register).
-		 * To change the rate we must disable the clock
-		 * first to cover these cases. Due to common
-		 * clock framework restrictions that do not allow
-		 * to disable a clock that has not been enabled,
-		 * we need to enable the clock first.
-		 */
-		ret = clk_prepare_enable(ctx->mclk);
-		if (!ret)
-			clk_disable_unprepare(ctx->mclk);
 
-		ret = clk_set_rate(ctx->mclk, CHT_PLAT_CLK_3_HZ);
+	/*
+	 * The firmware might enable the clock at
+	 * boot (this information may or may not
+	 * be reflected in the enable clock register).
+	 * To change the rate we must disable the clock
+	 * first to cover these cases. Due to common
+	 * clock framework restrictions that do not allow
+	 * to disable a clock that has not been enabled,
+	 * we need to enable the clock first.
+	 */
+	ret = clk_prepare_enable(ctx->mclk);
+	if (!ret)
+		clk_disable_unprepare(ctx->mclk);
+
+	ret = clk_set_rate(ctx->mclk, CHT_PLAT_CLK_3_HZ);
+
+	if (ret)
+		dev_err(runtime->dev, "unable to set MCLK rate\n");
 
-		if (ret)
-			dev_err(runtime->dev, "unable to set MCLK rate\n");
-	}
 	return ret;
 }
 
@@ -545,15 +542,15 @@ struct acpi_chan_package {   /* ACPICA seems to require 64 bit integers */
 
 static int snd_cht_mc_probe(struct platform_device *pdev)
 {
-	int ret_val = 0;
-	int i;
-	struct cht_mc_private *drv;
 	struct snd_soc_card *card = snd_soc_cards[0].soc_card;
 	struct sst_acpi_mach *mach;
+	struct cht_mc_private *drv;
 	const char *i2c_name = NULL;
-	int dai_index = 0;
 	bool found = false;
 	bool is_bytcr = false;
+	int dai_index = 0;
+	int ret_val = 0;
+	int i;
 
 	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_ATOMIC);
 	if (!drv)
@@ -590,7 +587,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 
 	/* fixup codec name based on HID */
 	i2c_name = sst_acpi_find_name_from_hid(mach->id);
-	if (i2c_name != NULL) {
+	if (i2c_name) {
 		snprintf(cht_rt5645_codec_name, sizeof(cht_rt5645_codec_name),
 			"%s%s", "i2c-", i2c_name);
 		cht_dailink[dai_index].codec_name = cht_rt5645_codec_name;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Applied "ASoC: Intel: bytcr_rt5640: cosmetic fixes" to the asoc tree
  2017-10-05 18:52 ` [PATCH 3/6] ASoC: Intel: bytcr_rt5640: cosmetic fixes Pierre-Louis Bossart
@ 2017-10-18 11:31   ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2017-10-18 11:31 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: alsa-devel, tiwai, Liam Girdwood, vinod.koul, broonie, Andy Shevchenko

The patch

   ASoC: Intel: bytcr_rt5640: cosmetic fixes

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 17b5273d847984fe8a7c3177554640fb7eac23da Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Date: Thu, 12 Oct 2017 18:37:58 -0500
Subject: [PATCH] ASoC: Intel: bytcr_rt5640: cosmetic fixes

Reorder variable names, change MCLK test, change quirks
No functional change

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Acked-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 73 +++++++++++++++++------------------
 1 file changed, 36 insertions(+), 37 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 7cee09d08ad7..ffb3ca971df4 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -22,12 +22,12 @@
 #include <linux/moduleparam.h>
 #include <linux/platform_device.h>
 #include <linux/acpi.h>
+#include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
 #include <asm/cpu_device_id.h>
 #include <asm/platform_sst_audio.h>
-#include <linux/clk.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
@@ -44,13 +44,13 @@ enum {
 	BYT_RT5640_IN3_MAP,
 };
 
-#define BYT_RT5640_MAP(quirk)	((quirk) & 0xff)
+#define BYT_RT5640_MAP(quirk)	((quirk) &  GENMASK(7, 0))
 #define BYT_RT5640_DMIC_EN	BIT(16)
 #define BYT_RT5640_MONO_SPEAKER BIT(17)
 #define BYT_RT5640_DIFF_MIC     BIT(18) /* defaut is single-ended */
-#define BYT_RT5640_SSP2_AIF2     BIT(19) /* default is using AIF1  */
-#define BYT_RT5640_SSP0_AIF1     BIT(20)
-#define BYT_RT5640_SSP0_AIF2     BIT(21)
+#define BYT_RT5640_SSP2_AIF2    BIT(19) /* default is using AIF1  */
+#define BYT_RT5640_SSP0_AIF1    BIT(20)
+#define BYT_RT5640_SSP0_AIF2    BIT(21)
 #define BYT_RT5640_MCLK_EN	BIT(22)
 #define BYT_RT5640_MCLK_25MHZ	BIT(23)
 
@@ -178,7 +178,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 	}
 
 	if (SND_SOC_DAPM_EVENT_ON(event)) {
-		if (priv->mclk) {
+		if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
 			ret = clk_prepare_enable(priv->mclk);
 			if (ret < 0) {
 				dev_err(card->dev,
@@ -199,7 +199,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 					     48000 * 512,
 					     SND_SOC_CLOCK_IN);
 		if (!ret) {
-			if (priv->mclk)
+			if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN)
 				clk_disable_unprepare(priv->mclk);
 		}
 	}
@@ -376,8 +376,8 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100TA"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN1_MAP |
-						 BYT_RT5640_MCLK_EN),
+		.driver_data = (void *)(BYT_RT5640_IN1_MAP |
+					BYT_RT5640_MCLK_EN),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -385,12 +385,11 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100TAF"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN1_MAP |
-						 BYT_RT5640_MONO_SPEAKER |
-						 BYT_RT5640_DIFF_MIC |
-						 BYT_RT5640_SSP0_AIF2 |
-						 BYT_RT5640_MCLK_EN
-						 ),
+		.driver_data = (void *)(BYT_RT5640_IN1_MAP |
+					BYT_RT5640_MONO_SPEAKER |
+					BYT_RT5640_DIFF_MIC |
+					BYT_RT5640_SSP0_AIF2 |
+					BYT_RT5640_MCLK_EN),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -398,9 +397,9 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "DellInc."),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Venue 8 Pro 5830"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_DMIC2_MAP |
-						 BYT_RT5640_DMIC_EN |
-						 BYT_RT5640_MCLK_EN),
+		.driver_data = (void *)(BYT_RT5640_DMIC2_MAP |
+					BYT_RT5640_DMIC_EN |
+					BYT_RT5640_MCLK_EN),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -408,8 +407,8 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "HP ElitePad 1000 G2"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN1_MAP |
-						 BYT_RT5640_MCLK_EN),
+		.driver_data = (void *)(BYT_RT5640_IN1_MAP |
+					BYT_RT5640_MCLK_EN),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -417,8 +416,8 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Circuitco"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "Minnowboard Max B3 PLATFORM"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_DMIC1_MAP |
-						 BYT_RT5640_DMIC_EN),
+		.driver_data = (void *)(BYT_RT5640_DMIC1_MAP |
+					BYT_RT5640_DMIC_EN),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -426,9 +425,9 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_MATCH(DMI_BOARD_VENDOR, "TECLAST"),
 			DMI_MATCH(DMI_BOARD_NAME, "tPAD"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN3_MAP |
-						BYT_RT5640_MCLK_EN |
-						BYT_RT5640_SSP0_AIF1),
+		.driver_data = (void *)(BYT_RT5640_IN3_MAP |
+					BYT_RT5640_MCLK_EN |
+					BYT_RT5640_SSP0_AIF1),
 	},
 	{
 		.callback = byt_rt5640_quirk_cb,
@@ -436,7 +435,7 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire SW5-012"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN1_MAP |
+		.driver_data = (void *)(BYT_RT5640_IN1_MAP |
 						 BYT_RT5640_MCLK_EN |
 						 BYT_RT5640_SSP0_AIF1),
 
@@ -446,9 +445,9 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Insyde"),
 		},
-		.driver_data = (unsigned long *)(BYT_RT5640_IN3_MAP |
-						 BYT_RT5640_MCLK_EN |
-						 BYT_RT5640_SSP0_AIF1),
+		.driver_data = (void *)(BYT_RT5640_IN3_MAP |
+					BYT_RT5640_MCLK_EN |
+					BYT_RT5640_SSP0_AIF1),
 
 	},
 	{}
@@ -456,12 +455,12 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 
 static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 {
-	int ret;
-	struct snd_soc_codec *codec = runtime->codec;
 	struct snd_soc_card *card = runtime->card;
-	const struct snd_soc_dapm_route *custom_map;
 	struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card);
+	struct snd_soc_codec *codec = runtime->codec;
+	const struct snd_soc_dapm_route *custom_map;
 	int num_routes;
+	int ret;
 
 	card->dapm.idle_bias_off = true;
 
@@ -549,7 +548,7 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 	snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone");
 	snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker");
 
-	if (priv->mclk) {
+	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
 		/*
 		 * The firmware might enable the clock at
 		 * boot (this information may or may not
@@ -758,12 +757,12 @@ struct acpi_chan_package {   /* ACPICA seems to require 64 bit integers */
 
 static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 {
-	int ret_val = 0;
+	struct byt_rt5640_private *priv;
 	struct sst_acpi_mach *mach;
 	const char *i2c_name = NULL;
-	int i;
+	int ret_val = 0;
 	int dai_index;
-	struct byt_rt5640_private *priv;
+	int i;
 
 	is_bytcr = false;
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC);
@@ -786,7 +785,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 
 	/* fixup codec name based on HID */
 	i2c_name = sst_acpi_find_name_from_hid(mach->id);
-	if (i2c_name != NULL) {
+	if (i2c_name) {
 		snprintf(byt_rt5640_codec_name, sizeof(byt_rt5640_codec_name),
 			"%s%s", "i2c-", i2c_name);
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-10-18 11:31 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-05 18:52 [PATCH 0/6] Baytrail-Cherrytrail updates Pierre-Louis Bossart
2017-10-05 18:52 ` [PATCH 1/6] ASoC: Intel: bytcr_rt5651: add MCLK support and fix quirks Pierre-Louis Bossart
2017-10-05 18:52 ` [PATCH 2/6] ASoC: Intel: bytcr_rt5651: filter codec name and fix variable declarations Pierre-Louis Bossart
2017-10-05 18:52 ` [PATCH 3/6] ASoC: Intel: bytcr_rt5640: cosmetic fixes Pierre-Louis Bossart
2017-10-18 11:31   ` Applied "ASoC: Intel: bytcr_rt5640: cosmetic fixes" to the asoc tree Mark Brown
2017-10-05 18:52 ` [PATCH 4/6] ASoC: Intel: cht_bsw_rt5645: cosmetic fixes Pierre-Louis Bossart
2017-10-18 11:31   ` Applied "ASoC: Intel: cht_bsw_rt5645: cosmetic fixes" to the asoc tree Mark Brown
2017-10-05 18:52 ` [PATCH 5/6] ASoC: Intel: bytcht_da7213: cosmetic fixes Pierre-Louis Bossart
2017-10-18 11:31   ` Applied "ASoC: Intel: bytcht_da7213: cosmetic fixes" to the asoc tree Mark Brown
2017-10-05 18:52 ` [PATCH 6/6] ASoC: Intel: boards: use strcmp with constant literals Pierre-Louis Bossart
2017-10-06  8:55   ` Takashi Iwai
2017-10-06 11:50     ` Andy Shevchenko
2017-10-06 14:28       ` Pierre-Louis Bossart
2017-10-06 15:24         ` Pierre-Louis Bossart
2017-10-06 15:48           ` Takashi Iwai
2017-10-09  9:34     ` Vinod Koul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.