[PATCH v2 0/4] ASoC: Intel: ACPI-related fixes

[PATCH v2 0/4] ASoC: Intel: ACPI-related fixes

[Pierre-Louis Bossart]

The first 3 patches were provided earlier on the mailing list or
in Bugzilla entries, I rebased and ported them on top of the latest
ACPI/KConfig fixes.
The last patch is a suggestion from Andy Shevchenko to remove
a hard-coded array size

Changes since v1:
 cleaned-up quirk handling, fixed merge mistake
 added Andy's Reviewed-by tag on patch 2
 added 4th patch to add new constant dependent on ACPI_ID_LEN

Jeremy Cline (1):
  ASoC: Replace snd_soc_acpi_check_hid with acpi_dev_present

Pierre-Louis Bossart (3):
  ASoC: acpi: fix machine driver selection based on quirk
  ASoC: Intel: bytcht_es8316: fix HID handling
  ASoC: acpi: remove hard-coded i2c-device name length

 include/sound/soc-acpi.h                |  6 ++---
 sound/soc/intel/boards/Kconfig          |  1 +
 sound/soc/intel/boards/bytcht_da7213.c  |  2 +-
 sound/soc/intel/boards/bytcht_es8316.c  | 26 ++++++++++++++++++++-
 sound/soc/intel/boards/bytcr_rt5640.c   |  2 +-
 sound/soc/intel/boards/bytcr_rt5651.c   |  2 +-
 sound/soc/intel/boards/cht_bsw_rt5645.c |  4 ++--
 sound/soc/intel/boards/cht_bsw_rt5672.c |  2 +-
 sound/soc/soc-acpi.c                    | 40 +++++----------------------------
 9 files changed, 40 insertions(+), 45 deletions(-)

-- 
2.14.1

[PATCH v2 1/4] ASoC: acpi: fix machine driver selection based on quirk

[Pierre-Louis Bossart]

The ACPI/machine-driver code refactoring introduced in 4.13 introduced
a regression for cases where we need a DMI-based quirk to select the
machine driver (the BIOS reports an invalid HID). The fix is just to
make sure the results of the quirk are actually used.

Fixes: 54746dabf770 ('ASoC: Improve machine driver selection based on quirk data')
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=96691
Tested-by: Nicole Færber <nicole.faerber@dpin.de>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/soc-acpi.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-acpi.c b/sound/soc/soc-acpi.c
index f21df28bc28e..d4dd2efea45e 100644
--- a/sound/soc/soc-acpi.c
+++ b/sound/soc/soc-acpi.c
@@ -84,11 +84,9 @@ snd_soc_acpi_find_machine(struct snd_soc_acpi_mach *machines)
 
 	for (mach = machines; mach->id[0]; mach++) {
 		if (snd_soc_acpi_check_hid(mach->id) == true) {
-			if (mach->machine_quirk == NULL)
-				return mach;
-
-			if (mach->machine_quirk(mach) != NULL)
-				return mach;
+			if (mach->machine_quirk)
+				mach = mach->machine_quirk(mach);
+			return mach;
 		}
 	}
 	return NULL;
-- 
2.14.1

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

Applied "ASoC: acpi: fix machine driver selection based on quirk" to the asoc tree

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2552 bytes --]

The patch

   ASoC: acpi: fix machine driver selection based on quirk

has been applied to the asoc tree at

   https://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 5c256045b87b8aa8e5bc9d2e2fdc0802351c1f99 Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Date: Fri, 5 Jan 2018 14:55:33 -0600
Subject: [PATCH] ASoC: acpi: fix machine driver selection based on quirk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The ACPI/machine-driver code refactoring introduced in 4.13 introduced
a regression for cases where we need a DMI-based quirk to select the
machine driver (the BIOS reports an invalid HID). The fix is just to
make sure the results of the quirk are actually used.

Fixes: 54746dabf770 ('ASoC: Improve machine driver selection based on quirk data')
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=96691
Tested-by: Nicole Færber <nicole.faerber@dpin.de>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 sound/soc/soc-acpi.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-acpi.c b/sound/soc/soc-acpi.c
index f21df28bc28e..d4dd2efea45e 100644
--- a/sound/soc/soc-acpi.c
+++ b/sound/soc/soc-acpi.c
@@ -84,11 +84,9 @@ snd_soc_acpi_find_machine(struct snd_soc_acpi_mach *machines)
 
 	for (mach = machines; mach->id[0]; mach++) {
 		if (snd_soc_acpi_check_hid(mach->id) == true) {
-			if (mach->machine_quirk == NULL)
-				return mach;
-
-			if (mach->machine_quirk(mach) != NULL)
-				return mach;
+			if (mach->machine_quirk)
+				mach = mach->machine_quirk(mach);
+			return mach;
 		}
 	}
 	return NULL;
-- 
2.15.1


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v2 2/4] ASoC: Replace snd_soc_acpi_check_hid with acpi_dev_present

[Pierre-Louis Bossart]

From: Jeremy Cline <jeremy@jcline.org>

Replace snd_soc_acpi_check_hid() with the generic acpi_dev_present()
and remove the now unused snd_soc_acpi_check_hid function. This should
have no functional change.

Signed-off-by: Jeremy Cline <jeremy@jcline.org>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/sound/soc-acpi.h |  3 ---
 sound/soc/soc-acpi.c     | 32 ++------------------------------
 2 files changed, 2 insertions(+), 33 deletions(-)

diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h
index a93436089bf5..d1aaf876cd26 100644
--- a/include/sound/soc-acpi.h
+++ b/include/sound/soc-acpi.h
@@ -50,9 +50,6 @@ snd_soc_acpi_find_package_from_hid(const u8 hid[ACPI_ID_LEN],
 struct snd_soc_acpi_mach *
 snd_soc_acpi_find_machine(struct snd_soc_acpi_mach *machines);
 
-/* acpi check hid */
-bool snd_soc_acpi_check_hid(const u8 hid[ACPI_ID_LEN]);
-
 /**
  * snd_soc_acpi_mach: ACPI-based machine descriptor. Most of the fields are
  * related to the hardware, except for the firmware and topology file names.
diff --git a/sound/soc/soc-acpi.c b/sound/soc/soc-acpi.c
index d4dd2efea45e..7f43c9bf3d09 100644
--- a/sound/soc/soc-acpi.c
+++ b/sound/soc/soc-acpi.c
@@ -49,41 +49,13 @@ const char *snd_soc_acpi_find_name_from_hid(const u8 hid[ACPI_ID_LEN])
 }
 EXPORT_SYMBOL_GPL(snd_soc_acpi_find_name_from_hid);
 
-static acpi_status snd_soc_acpi_mach_match(acpi_handle handle, u32 level,
-				       void *context, void **ret)
-{
-	unsigned long long sta;
-	acpi_status status;
-
-	*(bool *)context = true;
-	status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
-	if (ACPI_FAILURE(status) || !(sta & ACPI_STA_DEVICE_PRESENT))
-		*(bool *)context = false;
-
-	return AE_OK;
-}
-
-bool snd_soc_acpi_check_hid(const u8 hid[ACPI_ID_LEN])
-{
-	acpi_status status;
-	bool found = false;
-
-	status = acpi_get_devices(hid, snd_soc_acpi_mach_match, &found, NULL);
-
-	if (ACPI_FAILURE(status))
-		return false;
-
-	return found;
-}
-EXPORT_SYMBOL_GPL(snd_soc_acpi_check_hid);
-
 struct snd_soc_acpi_mach *
 snd_soc_acpi_find_machine(struct snd_soc_acpi_mach *machines)
 {
 	struct snd_soc_acpi_mach *mach;
 
 	for (mach = machines; mach->id[0]; mach++) {
-		if (snd_soc_acpi_check_hid(mach->id) == true) {
+		if (acpi_dev_present(mach->id, NULL, -1)) {
 			if (mach->machine_quirk)
 				mach = mach->machine_quirk(mach);
 			return mach;
@@ -161,7 +133,7 @@ struct snd_soc_acpi_mach *snd_soc_acpi_codec_list(void *arg)
 		return mach;
 
 	for (i = 0; i < codec_list->num_codecs; i++) {
-		if (snd_soc_acpi_check_hid(codec_list->codecs[i]) != true)
+		if (!acpi_dev_present(codec_list->codecs[i], NULL, -1))
 			return NULL;
 	}
 
-- 
2.14.1

Applied "ASoC: Replace snd_soc_acpi_check_hid with acpi_dev_present" to the asoc tree

[Mark Brown]

The patch

   ASoC: Replace snd_soc_acpi_check_hid with acpi_dev_present

has been applied to the asoc tree at

   https://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 0d5ea120abc020fada1f7cb019ec37f13162e7af Mon Sep 17 00:00:00 2001
From: Jeremy Cline <jeremy@jcline.org>
Date: Fri, 5 Jan 2018 14:55:34 -0600
Subject: [PATCH] ASoC: Replace snd_soc_acpi_check_hid with acpi_dev_present

Replace snd_soc_acpi_check_hid() with the generic acpi_dev_present()
and remove the now unused snd_soc_acpi_check_hid function. This should
have no functional change.

Signed-off-by: Jeremy Cline <jeremy@jcline.org>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/sound/soc-acpi.h |  3 ---
 sound/soc/soc-acpi.c     | 32 ++------------------------------
 2 files changed, 2 insertions(+), 33 deletions(-)

diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h
index a7d8d335b043..057805489af3 100644
--- a/include/sound/soc-acpi.h
+++ b/include/sound/soc-acpi.h
@@ -49,9 +49,6 @@ snd_soc_acpi_find_package_from_hid(const u8 hid[ACPI_ID_LEN],
 struct snd_soc_acpi_mach *
 snd_soc_acpi_find_machine(struct snd_soc_acpi_mach *machines);
 
-/* acpi check hid */
-bool snd_soc_acpi_check_hid(const u8 hid[ACPI_ID_LEN]);
-
 /**
  * snd_soc_acpi_mach: ACPI-based machine descriptor. Most of the fields are
  * related to the hardware, except for the firmware and topology file names.
diff --git a/sound/soc/soc-acpi.c b/sound/soc/soc-acpi.c
index d4dd2efea45e..7f43c9bf3d09 100644
--- a/sound/soc/soc-acpi.c
+++ b/sound/soc/soc-acpi.c
@@ -49,41 +49,13 @@ const char *snd_soc_acpi_find_name_from_hid(const u8 hid[ACPI_ID_LEN])
 }
 EXPORT_SYMBOL_GPL(snd_soc_acpi_find_name_from_hid);
 
-static acpi_status snd_soc_acpi_mach_match(acpi_handle handle, u32 level,
-				       void *context, void **ret)
-{
-	unsigned long long sta;
-	acpi_status status;
-
-	*(bool *)context = true;
-	status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
-	if (ACPI_FAILURE(status) || !(sta & ACPI_STA_DEVICE_PRESENT))
-		*(bool *)context = false;
-
-	return AE_OK;
-}
-
-bool snd_soc_acpi_check_hid(const u8 hid[ACPI_ID_LEN])
-{
-	acpi_status status;
-	bool found = false;
-
-	status = acpi_get_devices(hid, snd_soc_acpi_mach_match, &found, NULL);
-
-	if (ACPI_FAILURE(status))
-		return false;
-
-	return found;
-}
-EXPORT_SYMBOL_GPL(snd_soc_acpi_check_hid);
-
 struct snd_soc_acpi_mach *
 snd_soc_acpi_find_machine(struct snd_soc_acpi_mach *machines)
 {
 	struct snd_soc_acpi_mach *mach;
 
 	for (mach = machines; mach->id[0]; mach++) {
-		if (snd_soc_acpi_check_hid(mach->id) == true) {
+		if (acpi_dev_present(mach->id, NULL, -1)) {
 			if (mach->machine_quirk)
 				mach = mach->machine_quirk(mach);
 			return mach;
@@ -161,7 +133,7 @@ struct snd_soc_acpi_mach *snd_soc_acpi_codec_list(void *arg)
 		return mach;
 
 	for (i = 0; i < codec_list->num_codecs; i++) {
-		if (snd_soc_acpi_check_hid(codec_list->codecs[i]) != true)
+		if (!acpi_dev_present(codec_list->codecs[i], NULL, -1))
 			return NULL;
 	}
 
-- 
2.15.1

[PATCH v2 3/4] ASoC: Intel: bytcht_es8316: fix HID handling

[Pierre-Louis Bossart]

Same problem as with previous machine drivers, the codec dai
uses a hard-coded name of "i2c-ESSX8316:00" but ACPI provides
"i2c-ESSX8316:01" in some systems.

Fix by overriding the hard-coded value with the codec name derived
from the HID information

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=189261
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/boards/Kconfig         |  1 +
 sound/soc/intel/boards/bytcht_es8316.c | 26 +++++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index 89a73e3d9d2d..4af2393160bf 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -153,6 +153,7 @@ config SND_SOC_INTEL_BYT_CHT_DA7213_MACH
 config SND_SOC_INTEL_BYT_CHT_ES8316_MACH
 	tristate "Baytrail & Cherrytrail with ES8316 codec"
 	depends on X86_INTEL_LPSS && I2C && ACPI
+	select SND_SOC_ACPI
 	select SND_SOC_ES8316
 	help
 	  This adds support for ASoC machine driver for Intel(R) Baytrail &
diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
index 8088396717e3..ae24f6205f05 100644
--- a/sound/soc/intel/boards/bytcht_es8316.c
+++ b/sound/soc/intel/boards/bytcht_es8316.c
@@ -232,15 +232,39 @@ static struct snd_soc_card byt_cht_es8316_card = {
 	.fully_routed = true,
 };
 
+static char codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */
+
 static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
 {
-	int ret = 0;
 	struct byt_cht_es8316_private *priv;
+	struct snd_soc_acpi_mach *mach;
+	const char *i2c_name = NULL;
+	int dai_index = 0;
+	int i;
+	int ret = 0;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC);
 	if (!priv)
 		return -ENOMEM;
 
+	mach = (&pdev->dev)->platform_data;
+	/* fix index of codec dai */
+	for (i = 0; i < ARRAY_SIZE(byt_cht_es8316_dais); i++) {
+		if (!strcmp(byt_cht_es8316_dais[i].codec_name,
+			    "i2c-ESSX8316:00")) {
+			dai_index = i;
+			break;
+		}
+	}
+
+	/* fixup codec name based on HID */
+	i2c_name = snd_soc_acpi_find_name_from_hid(mach->id);
+	if (i2c_name) {
+		snprintf(codec_name, sizeof(codec_name),
+			"%s%s", "i2c-", i2c_name);
+		byt_cht_es8316_dais[dai_index].codec_name = codec_name;
+	}
+
 	/* register the soc card */
 	byt_cht_es8316_card.dev = &pdev->dev;
 	snd_soc_card_set_drvdata(&byt_cht_es8316_card, priv);
-- 
2.14.1

Re: [PATCH v2 3/4] ASoC: Intel: bytcht_es8316: fix HID handling

[Vinod Koul]

On Fri, Jan 05, 2018 at 02:55:35PM -0600, Pierre-Louis Bossart wrote:
> Same problem as with previous machine drivers, the codec dai
> uses a hard-coded name of "i2c-ESSX8316:00" but ACPI provides
> "i2c-ESSX8316:01" in some systems.
> 
> Fix by overriding the hard-coded value with the codec name derived
> from the HID information
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=189261
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  sound/soc/intel/boards/Kconfig         |  1 +
>  sound/soc/intel/boards/bytcht_es8316.c | 26 +++++++++++++++++++++++++-
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
> index 89a73e3d9d2d..4af2393160bf 100644
> --- a/sound/soc/intel/boards/Kconfig
> +++ b/sound/soc/intel/boards/Kconfig
> @@ -153,6 +153,7 @@ config SND_SOC_INTEL_BYT_CHT_DA7213_MACH
>  config SND_SOC_INTEL_BYT_CHT_ES8316_MACH
>  	tristate "Baytrail & Cherrytrail with ES8316 codec"
>  	depends on X86_INTEL_LPSS && I2C && ACPI
> +	select SND_SOC_ACPI
>  	select SND_SOC_ES8316
>  	help
>  	  This adds support for ASoC machine driver for Intel(R) Baytrail &
> diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
> index 8088396717e3..ae24f6205f05 100644
> --- a/sound/soc/intel/boards/bytcht_es8316.c
> +++ b/sound/soc/intel/boards/bytcht_es8316.c
> @@ -232,15 +232,39 @@ static struct snd_soc_card byt_cht_es8316_card = {
>  	.fully_routed = true,
>  };
>  
> +static char codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */
> +
>  static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
>  {
> -	int ret = 0;
>  	struct byt_cht_es8316_private *priv;
> +	struct snd_soc_acpi_mach *mach;
> +	const char *i2c_name = NULL;
> +	int dai_index = 0;
> +	int i;
> +	int ret = 0;
>  
>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC);
>  	if (!priv)
>  		return -ENOMEM;
>  
> +	mach = (&pdev->dev)->platform_data;
> +	/* fix index of codec dai */
> +	for (i = 0; i < ARRAY_SIZE(byt_cht_es8316_dais); i++) {
> +		if (!strcmp(byt_cht_es8316_dais[i].codec_name,
> +			    "i2c-ESSX8316:00")) {
> +			dai_index = i;
> +			break;
> +		}
> +	}
> +
> +	/* fixup codec name based on HID */
> +	i2c_name = snd_soc_acpi_find_name_from_hid(mach->id);
> +	if (i2c_name) {
> +		snprintf(codec_name, sizeof(codec_name),
> +			"%s%s", "i2c-", i2c_name);
> +		byt_cht_es8316_dais[dai_index].codec_name = codec_name;
> +	}

this looks good, but I though we had few other places where this was done,
esp the BSW based chromebooks, if so would it make send to have a macro in
soc-acpi which updates the dai name based on the result from
snd_soc_acpi_find_name_from_hid()

> +
>  	/* register the soc card */
>  	byt_cht_es8316_card.dev = &pdev->dev;
>  	snd_soc_card_set_drvdata(&byt_cht_es8316_card, priv);
> -- 
> 2.14.1
> 

-- 
~Vinod

Re: [PATCH v2 3/4] ASoC: Intel: bytcht_es8316: fix HID handling

[Pierre-Louis Bossart]

On 1/7/18 10:43 PM, Vinod Koul wrote:
> On Fri, Jan 05, 2018 at 02:55:35PM -0600, Pierre-Louis Bossart wrote:
>> Same problem as with previous machine drivers, the codec dai
>> uses a hard-coded name of "i2c-ESSX8316:00" but ACPI provides
>> "i2c-ESSX8316:01" in some systems.
>>
>> Fix by overriding the hard-coded value with the codec name derived
>> from the HID information
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=189261
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> ---
>>   sound/soc/intel/boards/Kconfig         |  1 +
>>   sound/soc/intel/boards/bytcht_es8316.c | 26 +++++++++++++++++++++++++-
>>   2 files changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
>> index 89a73e3d9d2d..4af2393160bf 100644
>> --- a/sound/soc/intel/boards/Kconfig
>> +++ b/sound/soc/intel/boards/Kconfig
>> @@ -153,6 +153,7 @@ config SND_SOC_INTEL_BYT_CHT_DA7213_MACH
>>   config SND_SOC_INTEL_BYT_CHT_ES8316_MACH
>>   	tristate "Baytrail & Cherrytrail with ES8316 codec"
>>   	depends on X86_INTEL_LPSS && I2C && ACPI
>> +	select SND_SOC_ACPI
>>   	select SND_SOC_ES8316
>>   	help
>>   	  This adds support for ASoC machine driver for Intel(R) Baytrail &
>> diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
>> index 8088396717e3..ae24f6205f05 100644
>> --- a/sound/soc/intel/boards/bytcht_es8316.c
>> +++ b/sound/soc/intel/boards/bytcht_es8316.c
>> @@ -232,15 +232,39 @@ static struct snd_soc_card byt_cht_es8316_card = {
>>   	.fully_routed = true,
>>   };
>>   
>> +static char codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */
>> +
>>   static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
>>   {
>> -	int ret = 0;
>>   	struct byt_cht_es8316_private *priv;
>> +	struct snd_soc_acpi_mach *mach;
>> +	const char *i2c_name = NULL;
>> +	int dai_index = 0;
>> +	int i;
>> +	int ret = 0;
>>   
>>   	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC);
>>   	if (!priv)
>>   		return -ENOMEM;
>>   
>> +	mach = (&pdev->dev)->platform_data;
>> +	/* fix index of codec dai */
>> +	for (i = 0; i < ARRAY_SIZE(byt_cht_es8316_dais); i++) {
>> +		if (!strcmp(byt_cht_es8316_dais[i].codec_name,
>> +			    "i2c-ESSX8316:00")) {
>> +			dai_index = i;
>> +			break;
>> +		}
>> +	}
>> +
>> +	/* fixup codec name based on HID */
>> +	i2c_name = snd_soc_acpi_find_name_from_hid(mach->id);
>> +	if (i2c_name) {
>> +		snprintf(codec_name, sizeof(codec_name),
>> +			"%s%s", "i2c-", i2c_name);
>> +		byt_cht_es8316_dais[dai_index].codec_name = codec_name;
>> +	}
> 
> this looks good, but I though we had few other places where this was done,
> esp the BSW based chromebooks, if so would it make send to have a macro in
> soc-acpi which updates the dai name based on the result from
> snd_soc_acpi_find_name_from_hid()

Well snd_soc_acpi_find_name_from_hid() will be replaced by an ACPI 
generic util (see proposal from Andy last week).

The idea was to add this first patch, and then do a replacement across 
all machine drivers when Andy's patch is available.

Andy also had another idea to add a helper which would take care of the 
for loop (which would indeed simplify the code further).

If that's alright with everyone, I'd like to add this patch first as is 
so that folks with the es8316 hardware get working audio, then do the 
two cleanups later.

> 
>> +
>>   	/* register the soc card */
>>   	byt_cht_es8316_card.dev = &pdev->dev;
>>   	snd_soc_card_set_drvdata(&byt_cht_es8316_card, priv);
>> -- 
>> 2.14.1
>>
> 

Re: [PATCH v2 3/4] ASoC: Intel: bytcht_es8316: fix HID handling

[Vinod Koul]

On Mon, Jan 08, 2018 at 02:23:19PM -0600, Pierre-Louis Bossart wrote:

> >>+	/* fixup codec name based on HID */
> >>+	i2c_name = snd_soc_acpi_find_name_from_hid(mach->id);
> >>+	if (i2c_name) {
> >>+		snprintf(codec_name, sizeof(codec_name),
> >>+			"%s%s", "i2c-", i2c_name);
> >>+		byt_cht_es8316_dais[dai_index].codec_name = codec_name;
> >>+	}
> >
> >this looks good, but I though we had few other places where this was done,
> >esp the BSW based chromebooks, if so would it make send to have a macro in
> >soc-acpi which updates the dai name based on the result from
> >snd_soc_acpi_find_name_from_hid()
> 
> Well snd_soc_acpi_find_name_from_hid() will be replaced by an ACPI generic
> util (see proposal from Andy last week).
> 
> The idea was to add this first patch, and then do a replacement across all
> machine drivers when Andy's patch is available.
> 
> Andy also had another idea to add a helper which would take care of the for
> loop (which would indeed simplify the code further).
> 
> If that's alright with everyone, I'd like to add this patch first as is so
> that folks with the es8316 hardware get working audio, then do the two
> cleanups later.

Yeah sounds fair to me :)

-- 
~Vinod

Re: [PATCH v2 3/4] ASoC: Intel: bytcht_es8316: fix HID handling

[Andy Shevchenko]

On Tue, 2018-01-09 at 10:18 +0530, Vinod Koul wrote:
> On Mon, Jan 08, 2018 at 02:23:19PM -0600, Pierre-Louis Bossart wrote:

> > Well snd_soc_acpi_find_name_from_hid() will be replaced by an ACPI
> > generic
> > util (see proposal from Andy last week).
> > 
> > The idea was to add this first patch, and then do a replacement
> > across all
> > machine drivers when Andy's patch is available.
> > 
> > Andy also had another idea to add a helper which would take care of
> > the for
> > loop (which would indeed simplify the code further).
> > 
> > If that's alright with everyone, I'd like to add this patch first as
> > is so
> > that folks with the es8316 hardware get working audio, then do the
> > two
> > cleanups later.
> 
> Yeah sounds fair to me :)

No objections from my side either.

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

[PATCH v2 4/4] ASoC: acpi: remove hard-coded i2c-device name length

[Pierre-Louis Bossart]

Remove hard-coded [16] array size, replace with clearer description and
dependency on ACPI_ID_LEN
No functionality change

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 include/sound/soc-acpi.h                | 3 +++
 sound/soc/intel/boards/bytcht_da7213.c  | 2 +-
 sound/soc/intel/boards/bytcht_es8316.c  | 2 +-
 sound/soc/intel/boards/bytcr_rt5640.c   | 2 +-
 sound/soc/intel/boards/bytcr_rt5651.c   | 2 +-
 sound/soc/intel/boards/cht_bsw_rt5645.c | 4 ++--
 sound/soc/intel/boards/cht_bsw_rt5672.c | 2 +-
 7 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h
index d1aaf876cd26..703c78483113 100644
--- a/include/sound/soc-acpi.h
+++ b/include/sound/soc-acpi.h
@@ -27,6 +27,9 @@ struct snd_soc_acpi_package_context {
 	bool data_valid;
 };
 
+/* codec name is used in DAIs is i2c-<HID>:00 with HID being 8 chars */
+#define SND_SOC_ACPI_I2C_DEVICE_NAME_LEN (4 + ACPI_ID_LEN + 3 + 1)
+
 #if IS_ENABLED(CONFIG_ACPI)
 /* translation fron HID to I2C name, needed for DAI codec_name */
 const char *snd_soc_acpi_find_name_from_hid(const u8 hid[ACPI_ID_LEN]);
diff --git a/sound/soc/intel/boards/bytcht_da7213.c b/sound/soc/intel/boards/bytcht_da7213.c
index c4d82ad41bd7..06c6a5deb071 100644
--- a/sound/soc/intel/boards/bytcht_da7213.c
+++ b/sound/soc/intel/boards/bytcht_da7213.c
@@ -219,7 +219,7 @@ static struct snd_soc_card bytcht_da7213_card = {
 	.num_dapm_routes = ARRAY_SIZE(audio_map),
 };
 
-static char codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */
+static char codec_name[SND_SOC_ACPI_I2C_DEVICE_NAME_LEN];
 
 static int bytcht_da7213_probe(struct platform_device *pdev)
 {
diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
index ae24f6205f05..0da04ebc118e 100644
--- a/sound/soc/intel/boards/bytcht_es8316.c
+++ b/sound/soc/intel/boards/bytcht_es8316.c
@@ -232,7 +232,7 @@ static struct snd_soc_card byt_cht_es8316_card = {
 	.fully_routed = true,
 };
 
-static char codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */
+static char codec_name[SND_SOC_ACPI_I2C_DEVICE_NAME_LEN];
 
 static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
 {
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index f2c0fc415e52..e7bbbc0eba55 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -713,7 +713,7 @@ static struct snd_soc_card byt_rt5640_card = {
 	.fully_routed = true,
 };
 
-static char byt_rt5640_codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */
+static char byt_rt5640_codec_name[SND_SOC_ACPI_I2C_DEVICE_NAME_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" */
 
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 488ec48f296a..23966105a494 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -481,7 +481,7 @@ 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 char byt_rt5651_codec_name[SND_SOC_ACPI_I2C_DEVICE_NAME_LEN];
 
 static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 {
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index f898ee140cdc..50acb55fe153 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -49,7 +49,7 @@ struct cht_acpi_card {
 struct cht_mc_private {
 	struct snd_soc_jack jack;
 	struct cht_acpi_card *acpi_card;
-	char codec_name[16];
+	char codec_name[SND_SOC_ACPI_I2C_DEVICE_NAME_LEN];
 	struct clk *mclk;
 };
 
@@ -506,7 +506,7 @@ static struct cht_acpi_card snd_soc_cards[] = {
 	{"10EC5650", CODEC_TYPE_RT5650, &snd_soc_card_chtrt5650},
 };
 
-static char cht_rt5645_codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */
+static char cht_rt5645_codec_name[SND_SOC_ACPI_I2C_DEVICE_NAME_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" */
 
diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c
index f8f21eee9b2d..b1decdacbae7 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5672.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5672.c
@@ -35,7 +35,7 @@
 
 struct cht_mc_private {
 	struct snd_soc_jack headset;
-	char codec_name[16];
+	char codec_name[SND_SOC_ACPI_I2C_DEVICE_NAME_LEN];
 	struct clk *mclk;
 };
 
-- 
2.14.1

Re: [PATCH v2 4/4] ASoC: acpi: remove hard-coded i2c-device name length

[Vinod Koul]

On Fri, Jan 05, 2018 at 02:55:36PM -0600, Pierre-Louis Bossart wrote:
> Remove hard-coded [16] array size, replace with clearer description and
> dependency on ACPI_ID_LEN
> No functionality change
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  include/sound/soc-acpi.h                | 3 +++
>  sound/soc/intel/boards/bytcht_da7213.c  | 2 +-
>  sound/soc/intel/boards/bytcht_es8316.c  | 2 +-
>  sound/soc/intel/boards/bytcr_rt5640.c   | 2 +-
>  sound/soc/intel/boards/bytcr_rt5651.c   | 2 +-
>  sound/soc/intel/boards/cht_bsw_rt5645.c | 4 ++--
>  sound/soc/intel/boards/cht_bsw_rt5672.c | 2 +-
>  7 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h
> index d1aaf876cd26..703c78483113 100644
> --- a/include/sound/soc-acpi.h
> +++ b/include/sound/soc-acpi.h
> @@ -27,6 +27,9 @@ struct snd_soc_acpi_package_context {
>  	bool data_valid;
>  };
>  
> +/* codec name is used in DAIs is i2c-<HID>:00 with HID being 8 chars */
> +#define SND_SOC_ACPI_I2C_DEVICE_NAME_LEN (4 + ACPI_ID_LEN + 3 + 1)

nitpicking, thats a very long name :(

how about SND_ACPI_I2C_ID_LEN, we can drop SOC. And replace DEVICE_NAME with ID

-- 
~Vinod

Re: [PATCH v2 4/4] ASoC: acpi: remove hard-coded i2c-device name length

[Pierre-Louis Bossart]

On 1/7/18 10:41 PM, Vinod Koul wrote:
> On Fri, Jan 05, 2018 at 02:55:36PM -0600, Pierre-Louis Bossart wrote:
>> Remove hard-coded [16] array size, replace with clearer description and
>> dependency on ACPI_ID_LEN
>> No functionality change
>>
>> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> ---
>>   include/sound/soc-acpi.h                | 3 +++
>>   sound/soc/intel/boards/bytcht_da7213.c  | 2 +-
>>   sound/soc/intel/boards/bytcht_es8316.c  | 2 +-
>>   sound/soc/intel/boards/bytcr_rt5640.c   | 2 +-
>>   sound/soc/intel/boards/bytcr_rt5651.c   | 2 +-
>>   sound/soc/intel/boards/cht_bsw_rt5645.c | 4 ++--
>>   sound/soc/intel/boards/cht_bsw_rt5672.c | 2 +-
>>   7 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h
>> index d1aaf876cd26..703c78483113 100644
>> --- a/include/sound/soc-acpi.h
>> +++ b/include/sound/soc-acpi.h
>> @@ -27,6 +27,9 @@ struct snd_soc_acpi_package_context {
>>   	bool data_valid;
>>   };
>>   
>> +/* codec name is used in DAIs is i2c-<HID>:00 with HID being 8 chars */
>> +#define SND_SOC_ACPI_I2C_DEVICE_NAME_LEN (4 + ACPI_ID_LEN + 3 + 1)
> 
> nitpicking, thats a very long name :(
> 
> how about SND_ACPI_I2C_ID_LEN, we can drop SOC. And replace DEVICE_NAME with ID

Fine with me. Andy do you concur or have a more precise suggestion?

Re: [PATCH v2 4/4] ASoC: acpi: remove hard-coded i2c-device name length

[Andy Shevchenko]

On Mon, 2018-01-08 at 14:28 -0600, Pierre-Louis Bossart wrote:
> On 1/7/18 10:41 PM, Vinod Koul wrote:
> > On Fri, Jan 05, 2018 at 02:55:36PM -0600, Pierre-Louis Bossart
> > wrote:
> > > Remove hard-coded [16] array size, replace with clearer
> > > description and
> > > dependency on ACPI_ID_LEN
> > > 


> > >   
> > > +/* codec name is used in DAIs is i2c-<HID>:00 with HID being 8
> > > chars */
> > > +#define SND_SOC_ACPI_I2C_DEVICE_NAME_LEN (4 + ACPI_ID_LEN + 3 +
> > > 1)
> > 
> > nitpicking, thats a very long name :(
> > 
> > how about SND_ACPI_I2C_ID_LEN, we can drop SOC. And replace
> > DEVICE_NAME with ID
> 
> Fine with me. Andy do you concur or have a more precise suggestion?

Looks sane to me.
Thank you, Vinod, for a suggestion.

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