[PATCH v3 00/22] ASoC: rt5651: jack-detect fixes and improvements

[PATCH v3 00/22] ASoC: rt5651: jack-detect fixes and improvements

[Hans de Goede]

Hi All,

Here is v3 of my rt5651 codec and Intel machine-driver series which mainly
consists of various jack-detect related fixes and improvements.

Changes in v3:
-Drop patches of which the v2 version has been merged (Thanks Mark)
-Split out dt-bindings documentation into a separate patch
-Rework how we deal with the machine-driver adding properties, so that
 there no longer is a need for calling the ugly private rt5651_apply_properties
 function from the machine-driver
-Split the patch for moving the configuration of the jack-detect source
 to device-properties into separate codec and machine-driver patches
-Split the patch making the OVCD threshold-current and scale-factor
 configurable into 2 separate patches

Changes in v2;
-Get rid of platform_data and use device-properties instead
-Split up a couple of the micbias / OVCD / jack-detect patches into
 smaller patches, as they were doing too much in a single patch
-Improved commit messages
-Concentrate the knowledge that SSP0 is 16 bits on BYTCR in the fixup
 function and extract the sample-format from the hw_params in other places

Regards,

Hans

[PATCH v3 01/22] ASoC: rt5651: Make rt5651_apply_properties() private

[Hans de Goede]

The idea behind exporting rt5651_apply_properties(), was for it to be used
on platforms where the platform code may need to add device-properties,
rather then relying only on properties set by the firmware. The platform
code could then call rt5651_apply_properties() after adding properties to
make sure that the codec driver was aware of the new properties.

But this is not necessary, as long as we do all property parsing from
the codec component-driver's probe function (or later) then the machine
driver can attach properties before calling snd_soc_register_card and
calling rt5651_apply_properties() for ordering reasons is not necessary.

This commit makes rt5651_apply_properties() private and adds 2 comments
documenting that all property parsing must be done from the codec
component-driver's probe function.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-This is a new patch in v3 of this patch-set
---
 sound/soc/codecs/rt5651.c | 14 ++++++++++++--
 sound/soc/codecs/rt5651.h |  2 --
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 5ea5233e1b7b..7bff5cda61fd 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1666,7 +1666,14 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	return 0;
 }
 
-void rt5651_apply_properties(struct snd_soc_component *component)
+/*
+ * Note on some platforms the platform code may need to add device-properties,
+ * rather then relying only on properties set by the firmware. Therefor the
+ * property parsing MUST be done from the component driver's probe function,
+ * rather then from the i2c driver's probe function, so that the platform-code
+ * can attach extra properties before calling snd_soc_register_card().
+ */
+static void rt5651_apply_properties(struct snd_soc_component *component)
 {
 	if (device_property_read_bool(component->dev, "realtek,in2-differential"))
 		snd_soc_component_update_bits(component, RT5651_IN1_IN2,
@@ -1676,7 +1683,6 @@ void rt5651_apply_properties(struct snd_soc_component *component)
 		snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
 				RT5651_GP2_PIN_MASK, RT5651_GP2_PIN_DMIC1_SCL);
 }
-EXPORT_SYMBOL_GPL(rt5651_apply_properties);
 
 static int rt5651_probe(struct snd_soc_component *component)
 {
@@ -1894,6 +1900,10 @@ static void rt5651_jack_detect_work(struct work_struct *work)
 	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
 }
 
+/*
+ * Note this function MUST not look at device-properties, see the comment
+ * above rt5651_apply_properties().
+ */
 static int rt5651_i2c_probe(struct i2c_client *i2c,
 		    const struct i2c_device_id *id)
 {
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 7d9d5fa09d6f..f3158488fc89 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2080,6 +2080,4 @@ struct rt5651_priv {
 	bool hp_mute;
 };
 
-void rt5651_apply_properties(struct snd_soc_component *component);
-
 #endif /* __RT5651_H__ */
-- 
2.14.3

Applied "ASoC: rt5651: Make rt5651_apply_properties() private" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Make rt5651_apply_properties() private

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 1cf5b50426136fe54380a7dd1ca7eb49973cae5a Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:35:49 +0100
Subject: [PATCH] ASoC: rt5651: Make rt5651_apply_properties() private

The idea behind exporting rt5651_apply_properties(), was for it to be used
on platforms where the platform code may need to add device-properties,
rather then relying only on properties set by the firmware. The platform
code could then call rt5651_apply_properties() after adding properties to
make sure that the codec driver was aware of the new properties.

But this is not necessary, as long as we do all property parsing from
the codec component-driver's probe function (or later) then the machine
driver can attach properties before calling snd_soc_register_card and
calling rt5651_apply_properties() for ordering reasons is not necessary.

This commit makes rt5651_apply_properties() private and adds 2 comments
documenting that all property parsing must be done from the codec
component-driver's probe function.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 14 ++++++++++++--
 sound/soc/codecs/rt5651.h |  2 --
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 55bcc74e344f..767a05e009df 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1666,7 +1666,14 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	return 0;
 }
 
-void rt5651_apply_properties(struct snd_soc_component *component)
+/*
+ * Note on some platforms the platform code may need to add device-properties,
+ * rather then relying only on properties set by the firmware. Therefor the
+ * property parsing MUST be done from the component driver's probe function,
+ * rather then from the i2c driver's probe function, so that the platform-code
+ * can attach extra properties before calling snd_soc_register_card().
+ */
+static void rt5651_apply_properties(struct snd_soc_component *component)
 {
 	if (device_property_read_bool(component->dev, "realtek,in2-differential"))
 		snd_soc_component_update_bits(component, RT5651_IN1_IN2,
@@ -1676,7 +1683,6 @@ void rt5651_apply_properties(struct snd_soc_component *component)
 		snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
 				RT5651_GP2_PIN_MASK, RT5651_GP2_PIN_DMIC1_SCL);
 }
-EXPORT_SYMBOL_GPL(rt5651_apply_properties);
 
 static int rt5651_probe(struct snd_soc_component *component)
 {
@@ -1893,6 +1899,10 @@ static void rt5651_jack_detect_work(struct work_struct *work)
 	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
 }
 
+/*
+ * Note this function MUST not look at device-properties, see the comment
+ * above rt5651_apply_properties().
+ */
 static int rt5651_i2c_probe(struct i2c_client *i2c,
 		    const struct i2c_device_id *id)
 {
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 7d9d5fa09d6f..f3158488fc89 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2080,6 +2080,4 @@ struct rt5651_priv {
 	bool hp_mute;
 };
 
-void rt5651_apply_properties(struct snd_soc_component *component);
-
 #endif /* __RT5651_H__ */
-- 
2.16.2

[PATCH v3 02/22] ASoC: rt5651: Add devicetree-bindings for jack-detect

[Hans de Goede]

Add new properties to the rt5651 dt-bindings for configuring the
jack-detect source and over-current detect settings.

Cc: devicetree@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/devicetree/bindings/sound/rt5651.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/rt5651.txt b/Documentation/devicetree/bindings/sound/rt5651.txt
index 3875233095f5..7890aa9477f8 100644
--- a/Documentation/devicetree/bindings/sound/rt5651.txt
+++ b/Documentation/devicetree/bindings/sound/rt5651.txt
@@ -16,6 +16,23 @@ Optional properties:
 - realtek,dmic-en
   Boolean. true if dmic is used.
 
+- realtek,jack-detect-source
+  u32. Valid values:
+  1: Use JD1_1 pin for jack-dectect
+  2: Use JD1_2 pin for jack-dectect
+  3: Use JD2 pin for jack-dectect
+
+- realtek,over-current-threshold-microamp
+  u32, micbias over-current detection threshold in µA, valid values are
+  600, 1500 and 2000µA.
+
+- realtek,over-current-scale-factor
+  u32, micbias over-current detection scale-factor, valid values are:
+  0: Scale current by 0.5
+  1: Scale current by 0.75
+  2: Scale current by 1.0
+  3: Scale current by 1.5
+
 Pins on the device (for linking into audio routes) for RT5651:
 
   * DMIC L1
-- 
2.14.3

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

Applied "ASoC: rt5651: Add devicetree-bindings for jack-detect" to the asoc tree

[Mark Brown]

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

The patch

   ASoC: rt5651: Add devicetree-bindings for jack-detect

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 9db7d4b32cbccb5807e2e1f22e2fafc80034ee6d Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:35:50 +0100
Subject: [PATCH] ASoC: rt5651: Add devicetree-bindings for jack-detect

Add new properties to the rt5651 dt-bindings for configuring the
jack-detect source and over-current detect settings.

Cc: devicetree@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/devicetree/bindings/sound/rt5651.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/rt5651.txt b/Documentation/devicetree/bindings/sound/rt5651.txt
index 3875233095f5..7890aa9477f8 100644
--- a/Documentation/devicetree/bindings/sound/rt5651.txt
+++ b/Documentation/devicetree/bindings/sound/rt5651.txt
@@ -16,6 +16,23 @@ Optional properties:
 - realtek,dmic-en
   Boolean. true if dmic is used.
 
+- realtek,jack-detect-source
+  u32. Valid values:
+  1: Use JD1_1 pin for jack-dectect
+  2: Use JD1_2 pin for jack-dectect
+  3: Use JD2 pin for jack-dectect
+
+- realtek,over-current-threshold-microamp
+  u32, micbias over-current detection threshold in µA, valid values are
+  600, 1500 and 2000µA.
+
+- realtek,over-current-scale-factor
+  u32, micbias over-current detection scale-factor, valid values are:
+  0: Scale current by 0.5
+  1: Scale current by 0.75
+  2: Scale current by 1.0
+  3: Scale current by 1.5
+
 Pins on the device (for linking into audio routes) for RT5651:
 
   * DMIC L1
-- 
2.16.2


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

[PATCH v3 03/22] ASoC: rt5651: Configure jack-detect source through a device-property

[Hans de Goede]

Configure the jack-detect source through a device-property which can be
set by code outside of the codec driver. Rather then putting platform
specific DMI quirks inside the generic codec driver.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3
-Split out the machine-driver and dt-bindings changes into separate patches,
 keeping just the codec driver changes in this patch

Changes in v2:
-New patch in v2 of this patch-set
---
 include/sound/rt5651.h    |  4 ++++
 sound/soc/codecs/rt5651.c | 33 +++++++--------------------------
 2 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
index 1612462bf5ad..725b36c329d0 100644
--- a/include/sound/rt5651.h
+++ b/include/sound/rt5651.h
@@ -11,6 +11,10 @@
 #ifndef __LINUX_SND_RT5651_H
 #define __LINUX_SND_RT5651_H
 
+/*
+ * Note these MUST match the values from the DT binding:
+ * Documentation/devicetree/bindings/sound/rt5651.txt
+ */
 enum rt5651_jd_src {
 	RT5651_JD_NULL,
 	RT5651_JD1_1,
diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 7bff5cda61fd..f80b3da4667f 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -19,7 +19,6 @@
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
 #include <linux/acpi.h>
-#include <linux/dmi.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -32,8 +31,6 @@
 #include "rl6231.h"
 #include "rt5651.h"
 
-#define RT5651_JD_MAP(quirk)	((quirk) & GENMASK(7, 0))
-
 #define RT5651_DEVICE_ID_VALUE 0x6281
 
 #define RT5651_PR_RANGE_BASE (0xff + 1)
@@ -41,8 +38,6 @@
 
 #define RT5651_PR_BASE (RT5651_PR_RANGE_BASE + (0 * RT5651_PR_SPACING))
 
-static unsigned long rt5651_quirk;
-
 static const struct regmap_range_cfg rt5651_ranges[] = {
 	{ .name = "PR", .range_min = RT5651_PR_BASE,
 	  .range_max = RT5651_PR_BASE + 0xb4,
@@ -1675,6 +1670,9 @@ static int rt5651_set_jack(struct snd_soc_component *component,
  */
 static void rt5651_apply_properties(struct snd_soc_component *component)
 {
+	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
+	u32 val;
+
 	if (device_property_read_bool(component->dev, "realtek,in2-differential"))
 		snd_soc_component_update_bits(component, RT5651_IN1_IN2,
 				RT5651_IN_DF2, RT5651_IN_DF2);
@@ -1682,6 +1680,10 @@ static void rt5651_apply_properties(struct snd_soc_component *component)
 	if (device_property_read_bool(component->dev, "realtek,dmic-en"))
 		snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
 				RT5651_GP2_PIN_MASK, RT5651_GP2_PIN_DMIC1_SCL);
+
+	if (device_property_read_u32(component->dev,
+				     "realtek,jack-detect-source", &val) == 0)
+		rt5651->jd_src = val;
 }
 
 static int rt5651_probe(struct snd_soc_component *component)
@@ -1832,24 +1834,6 @@ static const struct i2c_device_id rt5651_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rt5651_i2c_id);
 
-static int rt5651_quirk_cb(const struct dmi_system_id *id)
-{
-	rt5651_quirk = (unsigned long) id->driver_data;
-	return 1;
-}
-
-static const struct dmi_system_id rt5651_quirk_table[] = {
-	{
-		.callback = rt5651_quirk_cb,
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "KIANO"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "KIANO SlimNote 14.2"),
-		},
-		.driver_data = (unsigned long *) RT5651_JD1_1,
-	},
-	{}
-};
-
 static int rt5651_jack_detect(struct snd_soc_component *component, int jack_insert)
 {
 	int jack_type;
@@ -1917,9 +1901,6 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 
 	i2c_set_clientdata(i2c, rt5651);
 
-	dmi_check_system(rt5651_quirk_table);
-	rt5651->jd_src = RT5651_JD_MAP(rt5651_quirk);
-
 	rt5651->regmap = devm_regmap_init_i2c(i2c, &rt5651_regmap);
 	if (IS_ERR(rt5651->regmap)) {
 		ret = PTR_ERR(rt5651->regmap);
-- 
2.14.3

Re: [PATCH v3 03/22] ASoC: rt5651: Configure jack-detect source through a device-property

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 869 bytes --]

On Sun, Mar 04, 2018 at 03:35:51PM +0100, Hans de Goede wrote:

> +/*
> + * Note these MUST match the values from the DT binding:
> + * Documentation/devicetree/bindings/sound/rt5651.txt
> + */
>  enum rt5651_jd_src {
>  	RT5651_JD_NULL,
>  	RT5651_JD1_1,

It's normal to put values like this in a header in include/dt-bindings
so it's easy to get things right in actual DT bindings (and possibly
ACPI tables as well if the machinery for that does the right thing) -
can you send a followup doing that please?

> -static const struct dmi_system_id rt5651_quirk_table[] = {
> -	{
> -		.callback = rt5651_quirk_cb,
> -		.matches = {
> -			DMI_MATCH(DMI_SYS_VENDOR, "KIANO"),
> -			DMI_MATCH(DMI_PRODUCT_NAME, "KIANO SlimNote 14.2"),
> -		},
> -		.driver_data = (unsigned long *) RT5651_JD1_1,
> -	},
> -	{}
> -};
> -

I'm still not thrilled about this bit but oh well :(

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

Re: [PATCH v3 03/22] ASoC: rt5651: Configure jack-detect source through a device-property

[Hans de Goede]

Hi,

On 07-03-18 13:34, Mark Brown wrote:
> On Sun, Mar 04, 2018 at 03:35:51PM +0100, Hans de Goede wrote:
> 
>> +/*
>> + * Note these MUST match the values from the DT binding:
>> + * Documentation/devicetree/bindings/sound/rt5651.txt
>> + */
>>   enum rt5651_jd_src {
>>   	RT5651_JD_NULL,
>>   	RT5651_JD1_1,
> 
> It's normal to put values like this in a header in include/dt-bindings
> so it's easy to get things right in actual DT bindings (and possibly
> ACPI tables as well if the machinery for that does the right thing) -
> can you send a followup doing that please?

Good point, yes I will send a follow-up patch.

>> -static const struct dmi_system_id rt5651_quirk_table[] = {
>> -	{
>> -		.callback = rt5651_quirk_cb,
>> -		.matches = {
>> -			DMI_MATCH(DMI_SYS_VENDOR, "KIANO"),
>> -			DMI_MATCH(DMI_PRODUCT_NAME, "KIANO SlimNote 14.2"),
>> -		},
>> -		.driver_data = (unsigned long *) RT5651_JD1_1,
>> -	},
>> -	{}
>> -};
>> -
> 
> I'm still not thrilled about this bit but oh well :(

Hmm, I'm planning on doing something very similar for the rt5640 code.

That is I'm working on jack-detect for the rt5640 too, and that will
need some platform data like which pin to use and OVCD settings,
I plan to once again use device-properties for this to have a
platform agnostic solution at the codec driver level.

And then in the Intel machine driver attach the device properties
based on dmi quirks (like bytcr_rt5651 the machine driver already
has DMI based quirks for a bunch of boards, which I will re-use).

Although I agree that ideally we would be able to get all this
directly from the firmware, to me fixing the x86 specific problem
of that info missing using quirks in the Intel machine driver seems
like the right place to fix this, so that we are not "poluting"
generic code with these Intel specific workarounds.

I know that second best would be some generic mechanism in the
kernel to attach extra device-properties based on the board we're
booting on, but alas currently that does not exist. One advantage
of using device-properties for this as this series is doing, is
that if such a mechanism gets added moving the info over should
be easy.

TL;DR: I know this is not the prettiest thing ever, but as you
said: "oh well".

So as said I plan to do something very similar for the rt5640 code,
if you've any objections against that I would like to hear those
objections sooner rather then later to avoid throwing away work.

Regards,

Hans

Re: [PATCH v3 03/22] ASoC: rt5651: Configure jack-detect source through a device-property

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 651 bytes --]

On Wed, Mar 07, 2018 at 02:49:05PM +0100, Hans de Goede wrote:

> Although I agree that ideally we would be able to get all this
> directly from the firmware, to me fixing the x86 specific problem
> of that info missing using quirks in the Intel machine driver seems
> like the right place to fix this, so that we are not "poluting"
> generic code with these Intel specific workarounds.

Yeah, my feeling here is that this is less pollution than you're seeing
it as - it's too much like endorsement of the Windows one driver per
machine way of doing things and splits the knowledge about how the
device gets used about more.  Purely a taste question.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

Applied "ASoC: rt5651: Configure jack-detect source through a device-property" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Configure jack-detect source through a device-property

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 f0c2a330d99ef81519dc809d8b6a7dafe39b0933 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:35:51 +0100
Subject: [PATCH] ASoC: rt5651: Configure jack-detect source through a
 device-property

Configure the jack-detect source through a device-property which can be
set by code outside of the codec driver. Rather then putting platform
specific DMI quirks inside the generic codec driver.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/sound/rt5651.h    |  4 ++++
 sound/soc/codecs/rt5651.c | 33 +++++++--------------------------
 2 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
index 1612462bf5ad..725b36c329d0 100644
--- a/include/sound/rt5651.h
+++ b/include/sound/rt5651.h
@@ -11,6 +11,10 @@
 #ifndef __LINUX_SND_RT5651_H
 #define __LINUX_SND_RT5651_H
 
+/*
+ * Note these MUST match the values from the DT binding:
+ * Documentation/devicetree/bindings/sound/rt5651.txt
+ */
 enum rt5651_jd_src {
 	RT5651_JD_NULL,
 	RT5651_JD1_1,
diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 767a05e009df..50e1c501b6b9 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -19,7 +19,6 @@
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
 #include <linux/acpi.h>
-#include <linux/dmi.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -32,8 +31,6 @@
 #include "rl6231.h"
 #include "rt5651.h"
 
-#define RT5651_JD_MAP(quirk)	((quirk) & GENMASK(7, 0))
-
 #define RT5651_DEVICE_ID_VALUE 0x6281
 
 #define RT5651_PR_RANGE_BASE (0xff + 1)
@@ -41,8 +38,6 @@
 
 #define RT5651_PR_BASE (RT5651_PR_RANGE_BASE + (0 * RT5651_PR_SPACING))
 
-static unsigned long rt5651_quirk;
-
 static const struct regmap_range_cfg rt5651_ranges[] = {
 	{ .name = "PR", .range_min = RT5651_PR_BASE,
 	  .range_max = RT5651_PR_BASE + 0xb4,
@@ -1675,6 +1670,9 @@ static int rt5651_set_jack(struct snd_soc_component *component,
  */
 static void rt5651_apply_properties(struct snd_soc_component *component)
 {
+	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
+	u32 val;
+
 	if (device_property_read_bool(component->dev, "realtek,in2-differential"))
 		snd_soc_component_update_bits(component, RT5651_IN1_IN2,
 				RT5651_IN_DF2, RT5651_IN_DF2);
@@ -1682,6 +1680,10 @@ static void rt5651_apply_properties(struct snd_soc_component *component)
 	if (device_property_read_bool(component->dev, "realtek,dmic-en"))
 		snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
 				RT5651_GP2_PIN_MASK, RT5651_GP2_PIN_DMIC1_SCL);
+
+	if (device_property_read_u32(component->dev,
+				     "realtek,jack-detect-source", &val) == 0)
+		rt5651->jd_src = val;
 }
 
 static int rt5651_probe(struct snd_soc_component *component)
@@ -1831,24 +1833,6 @@ static const struct i2c_device_id rt5651_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rt5651_i2c_id);
 
-static int rt5651_quirk_cb(const struct dmi_system_id *id)
-{
-	rt5651_quirk = (unsigned long) id->driver_data;
-	return 1;
-}
-
-static const struct dmi_system_id rt5651_quirk_table[] = {
-	{
-		.callback = rt5651_quirk_cb,
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "KIANO"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "KIANO SlimNote 14.2"),
-		},
-		.driver_data = (unsigned long *) RT5651_JD1_1,
-	},
-	{}
-};
-
 static int rt5651_jack_detect(struct snd_soc_component *component, int jack_insert)
 {
 	int jack_type;
@@ -1916,9 +1900,6 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 
 	i2c_set_clientdata(i2c, rt5651);
 
-	dmi_check_system(rt5651_quirk_table);
-	rt5651->jd_src = RT5651_JD_MAP(rt5651_quirk);
-
 	rt5651->regmap = devm_regmap_init_i2c(i2c, &rt5651_regmap);
 	if (IS_ERR(rt5651->regmap)) {
 		ret = PTR_ERR(rt5651->regmap);
-- 
2.16.2

[PATCH v3 04/22] ASoC: rt5651: Allow specifying over-current threshold through a device-property

[Hans de Goede]

OVer-Current-Detection (OVCD) for the micbias current is used to detect
if an inserted jack is a headset or headphones (mic shorted to ground).

Some boards may need different values for the OVCD current threshold
because of a resistor on the board in serial with or parallel to the
jack mic contact.

This commit adds support for configuring the OCVD current threshold
through the "realtek,over-current-threshold-microamp" device-property.

Note this commit changes the default value from 600uA to 2000uA,
because testing has shown 600uA to be a poor default.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-Split out the dt-bindings documentation for the new properties into a
 separate patch
-Append "-microamp to the device-property name
-Split out seeting of OCVD scale-factor into a separate patch, keeping
 only OVCD current threshold configuration in this patch
---
 sound/soc/codecs/rt5651.c | 22 +++++++++++++++++++++-
 sound/soc/codecs/rt5651.h |  1 +
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index f80b3da4667f..19f7c89d1e7a 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1638,7 +1638,7 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 				      RT5651_PWR_CLK12M_MASK |
 				      RT5651_PWR_MB_MASK,
 				      RT5651_MIC1_OVCD_EN |
-				      RT5651_MIC1_OVTH_600UA |
+				      rt5651->ovcd_th |
 				      RT5651_PWR_MB_PU |
 				      RT5651_PWR_CLK12M_PU);
 
@@ -1684,6 +1684,26 @@ static void rt5651_apply_properties(struct snd_soc_component *component)
 	if (device_property_read_u32(component->dev,
 				     "realtek,jack-detect-source", &val) == 0)
 		rt5651->jd_src = val;
+
+	rt5651->ovcd_th = RT5651_MIC1_OVTH_2000UA;
+
+	if (device_property_read_u32(component->dev,
+			"realtek,over-current-threshold-microamp", &val) == 0) {
+		switch (val) {
+		case 600:
+			rt5651->ovcd_th = RT5651_MIC1_OVTH_600UA;
+			break;
+		case 1500:
+			rt5651->ovcd_th = RT5651_MIC1_OVTH_1500UA;
+			break;
+		case 2000:
+			rt5651->ovcd_th = RT5651_MIC1_OVTH_2000UA;
+			break;
+		default:
+			dev_warn(component->dev, "Warning: Invalid over-current-threshold-microamp value: %d, defaulting to 2000uA\n",
+				 val);
+		}
+	}
 }
 
 static int rt5651_probe(struct snd_soc_component *component)
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index f3158488fc89..9cd5c279d0d6 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2064,6 +2064,7 @@ struct rt5651_priv {
 	struct snd_soc_jack *hp_jack;
 	struct delayed_work jack_detect_work;
 	enum rt5651_jd_src jd_src;
+	unsigned int ovcd_th;
 
 	int irq;
 	int sysclk;
-- 
2.14.3

Applied "ASoC: rt5651: Allow specifying over-current threshold through a device-property" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Allow specifying over-current threshold through a device-property

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 583a9debd71e21a317b1fc7b293c22c49b33d9e4 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:35:52 +0100
Subject: [PATCH] ASoC: rt5651: Allow specifying over-current threshold through
 a device-property

OVer-Current-Detection (OVCD) for the micbias current is used to detect
if an inserted jack is a headset or headphones (mic shorted to ground).

Some boards may need different values for the OVCD current threshold
because of a resistor on the board in serial with or parallel to the
jack mic contact.

This commit adds support for configuring the OCVD current threshold
through the "realtek,over-current-threshold-microamp" device-property.

Note this commit changes the default value from 600uA to 2000uA,
because testing has shown 600uA to be a poor default.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 22 +++++++++++++++++++++-
 sound/soc/codecs/rt5651.h |  1 +
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 50e1c501b6b9..7ff1bc892cfd 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1638,7 +1638,7 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 				      RT5651_PWR_CLK12M_MASK |
 				      RT5651_PWR_MB_MASK,
 				      RT5651_MIC1_OVCD_EN |
-				      RT5651_MIC1_OVTH_600UA |
+				      rt5651->ovcd_th |
 				      RT5651_PWR_MB_PU |
 				      RT5651_PWR_CLK12M_PU);
 
@@ -1684,6 +1684,26 @@ static void rt5651_apply_properties(struct snd_soc_component *component)
 	if (device_property_read_u32(component->dev,
 				     "realtek,jack-detect-source", &val) == 0)
 		rt5651->jd_src = val;
+
+	rt5651->ovcd_th = RT5651_MIC1_OVTH_2000UA;
+
+	if (device_property_read_u32(component->dev,
+			"realtek,over-current-threshold-microamp", &val) == 0) {
+		switch (val) {
+		case 600:
+			rt5651->ovcd_th = RT5651_MIC1_OVTH_600UA;
+			break;
+		case 1500:
+			rt5651->ovcd_th = RT5651_MIC1_OVTH_1500UA;
+			break;
+		case 2000:
+			rt5651->ovcd_th = RT5651_MIC1_OVTH_2000UA;
+			break;
+		default:
+			dev_warn(component->dev, "Warning: Invalid over-current-threshold-microamp value: %d, defaulting to 2000uA\n",
+				 val);
+		}
+	}
 }
 
 static int rt5651_probe(struct snd_soc_component *component)
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index f3158488fc89..9cd5c279d0d6 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2064,6 +2064,7 @@ struct rt5651_priv {
 	struct snd_soc_jack *hp_jack;
 	struct delayed_work jack_detect_work;
 	enum rt5651_jd_src jd_src;
+	unsigned int ovcd_th;
 
 	int irq;
 	int sysclk;
-- 
2.16.2

[PATCH v3 05/22] ASoC: rt5651: Allow specifying the OVCD scale-factor through a device-property

[Hans de Goede]

OVer-Current-Detection (OVCD) for the micbias current is used to detect if
an inserted jack is a headset or headphones (mic shorted to ground).

The threshold for at which current the OVCD triggers on the rt5651 is not
only controlled by setting the absolute current limit, but also by setting
a scale factor which applies to the limit. Testing has shown that we need
to set both (depending on the board).

This commit adds support for the sofar unused OVCD scale-factor register
and adds support for specifying non-default values for it through the
"realtek,over-current-scale-factor" device-property.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-Split out the changes adding OVCD-scale-factor support from those making
 the OVCD-current-limit configurable
---
 include/sound/rt5651.h    | 11 +++++++++++
 sound/soc/codecs/rt5651.c | 19 +++++++++++++++++++
 sound/soc/codecs/rt5651.h | 11 +++++++++++
 3 files changed, 41 insertions(+)

diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
index 725b36c329d0..6403b862fb9a 100644
--- a/include/sound/rt5651.h
+++ b/include/sound/rt5651.h
@@ -22,4 +22,15 @@ enum rt5651_jd_src {
 	RT5651_JD2,
 };
 
+/*
+ * Note these MUST match the values from the DT binding:
+ * Documentation/devicetree/bindings/sound/rt5651.txt
+ */
+enum rt5651_ovcd_sf {
+	RT5651_OVCD_SF_0P5,
+	RT5651_OVCD_SF_0P75,
+	RT5651_OVCD_SF_1P0,
+	RT5651_OVCD_SF_1P5,
+};
+
 #endif
diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 19f7c89d1e7a..4f78694adf70 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1632,6 +1632,10 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
 		RT5651_PWR_JD_M, RT5651_PWR_JD_M);
 
+	/* Set OVCD threshold current and scale-factor */
+	snd_soc_component_write(component, RT5651_PR_BASE + RT5651_BIAS_CUR4,
+				0xa800 | rt5651->ovcd_sf);
+
 	snd_soc_component_update_bits(component, RT5651_MICBIAS,
 				      RT5651_MIC1_OVCD_MASK |
 				      RT5651_MIC1_OVTH_MASK |
@@ -1685,7 +1689,13 @@ static void rt5651_apply_properties(struct snd_soc_component *component)
 				     "realtek,jack-detect-source", &val) == 0)
 		rt5651->jd_src = val;
 
+	/*
+	 * Testing on various boards has shown that good defaults for the OVCD
+	 * threshold and scale-factor are 2000µA and 0.75. For an effective
+	 * limit of 1500µA, this seems to be more reliable then 1500µA and 1.0.
+	 */
 	rt5651->ovcd_th = RT5651_MIC1_OVTH_2000UA;
+	rt5651->ovcd_sf = RT5651_MIC_OVCD_SF_0P75;
 
 	if (device_property_read_u32(component->dev,
 			"realtek,over-current-threshold-microamp", &val) == 0) {
@@ -1704,6 +1714,15 @@ static void rt5651_apply_properties(struct snd_soc_component *component)
 				 val);
 		}
 	}
+
+	if (device_property_read_u32(component->dev,
+			"realtek,over-current-scale-factor", &val) == 0) {
+		if (val <= RT5651_OVCD_SF_1P5)
+			rt5651->ovcd_sf = val << RT5651_MIC_OVCD_SF_SFT;
+		else
+			dev_warn(component->dev, "Warning: Invalid over-current-scale-factor value: %d, defaulting to 0.75\n",
+				 val);
+	}
 }
 
 static int rt5651_probe(struct snd_soc_component *component)
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 9cd5c279d0d6..71738ab93fb9 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -138,6 +138,7 @@
 /* Index of Codec Private Register definition */
 #define RT5651_BIAS_CUR1			0x12
 #define RT5651_BIAS_CUR3			0x14
+#define RT5651_BIAS_CUR4			0x15
 #define RT5651_CLSD_INT_REG1			0x1c
 #define RT5651_CHPUMP_INT_REG1			0x24
 #define RT5651_MAMP_INT_REG2			0x37
@@ -1966,6 +1967,15 @@
 #define RT5651_D_GATE_EN_SFT			0
 
 /* Codec Private Register definition */
+
+/* MIC Over current threshold scale factor (0x15) */
+#define RT5651_MIC_OVCD_SF_MASK			(0x3 << 8)
+#define RT5651_MIC_OVCD_SF_SFT			8
+#define RT5651_MIC_OVCD_SF_0P5			(0x0 << 8)
+#define RT5651_MIC_OVCD_SF_0P75			(0x1 << 8)
+#define RT5651_MIC_OVCD_SF_1P0			(0x2 << 8)
+#define RT5651_MIC_OVCD_SF_1P5			(0x3 << 8)
+
 /* 3D Speaker Control (0x63) */
 #define RT5651_3D_SPK_MASK			(0x1 << 15)
 #define RT5651_3D_SPK_SFT			15
@@ -2065,6 +2075,7 @@ struct rt5651_priv {
 	struct delayed_work jack_detect_work;
 	enum rt5651_jd_src jd_src;
 	unsigned int ovcd_th;
+	unsigned int ovcd_sf;
 
 	int irq;
 	int sysclk;
-- 
2.14.3

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

Applied "ASoC: rt5651: Allow specifying the OVCD scale-factor through a device-property" to the asoc tree

[Mark Brown]

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

The patch

   ASoC: rt5651: Allow specifying the OVCD scale-factor through a device-property

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 e6eb0207597afa1cdd4914a17a727b101cc859ff Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:35:53 +0100
Subject: [PATCH] ASoC: rt5651: Allow specifying the OVCD scale-factor through
 a device-property

OVer-Current-Detection (OVCD) for the micbias current is used to detect if
an inserted jack is a headset or headphones (mic shorted to ground).

The threshold for at which current the OVCD triggers on the rt5651 is not
only controlled by setting the absolute current limit, but also by setting
a scale factor which applies to the limit. Testing has shown that we need
to set both (depending on the board).

This commit adds support for the sofar unused OVCD scale-factor register
and adds support for specifying non-default values for it through the
"realtek,over-current-scale-factor" device-property.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/sound/rt5651.h    | 11 +++++++++++
 sound/soc/codecs/rt5651.c | 19 +++++++++++++++++++
 sound/soc/codecs/rt5651.h | 11 +++++++++++
 3 files changed, 41 insertions(+)

diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
index 725b36c329d0..6403b862fb9a 100644
--- a/include/sound/rt5651.h
+++ b/include/sound/rt5651.h
@@ -22,4 +22,15 @@ enum rt5651_jd_src {
 	RT5651_JD2,
 };
 
+/*
+ * Note these MUST match the values from the DT binding:
+ * Documentation/devicetree/bindings/sound/rt5651.txt
+ */
+enum rt5651_ovcd_sf {
+	RT5651_OVCD_SF_0P5,
+	RT5651_OVCD_SF_0P75,
+	RT5651_OVCD_SF_1P0,
+	RT5651_OVCD_SF_1P5,
+};
+
 #endif
diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 7ff1bc892cfd..486817809b7b 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1632,6 +1632,10 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
 		RT5651_PWR_JD_M, RT5651_PWR_JD_M);
 
+	/* Set OVCD threshold current and scale-factor */
+	snd_soc_component_write(component, RT5651_PR_BASE + RT5651_BIAS_CUR4,
+				0xa800 | rt5651->ovcd_sf);
+
 	snd_soc_component_update_bits(component, RT5651_MICBIAS,
 				      RT5651_MIC1_OVCD_MASK |
 				      RT5651_MIC1_OVTH_MASK |
@@ -1685,7 +1689,13 @@ static void rt5651_apply_properties(struct snd_soc_component *component)
 				     "realtek,jack-detect-source", &val) == 0)
 		rt5651->jd_src = val;
 
+	/*
+	 * Testing on various boards has shown that good defaults for the OVCD
+	 * threshold and scale-factor are 2000µA and 0.75. For an effective
+	 * limit of 1500µA, this seems to be more reliable then 1500µA and 1.0.
+	 */
 	rt5651->ovcd_th = RT5651_MIC1_OVTH_2000UA;
+	rt5651->ovcd_sf = RT5651_MIC_OVCD_SF_0P75;
 
 	if (device_property_read_u32(component->dev,
 			"realtek,over-current-threshold-microamp", &val) == 0) {
@@ -1704,6 +1714,15 @@ static void rt5651_apply_properties(struct snd_soc_component *component)
 				 val);
 		}
 	}
+
+	if (device_property_read_u32(component->dev,
+			"realtek,over-current-scale-factor", &val) == 0) {
+		if (val <= RT5651_OVCD_SF_1P5)
+			rt5651->ovcd_sf = val << RT5651_MIC_OVCD_SF_SFT;
+		else
+			dev_warn(component->dev, "Warning: Invalid over-current-scale-factor value: %d, defaulting to 0.75\n",
+				 val);
+	}
 }
 
 static int rt5651_probe(struct snd_soc_component *component)
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 9cd5c279d0d6..71738ab93fb9 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -138,6 +138,7 @@
 /* Index of Codec Private Register definition */
 #define RT5651_BIAS_CUR1			0x12
 #define RT5651_BIAS_CUR3			0x14
+#define RT5651_BIAS_CUR4			0x15
 #define RT5651_CLSD_INT_REG1			0x1c
 #define RT5651_CHPUMP_INT_REG1			0x24
 #define RT5651_MAMP_INT_REG2			0x37
@@ -1966,6 +1967,15 @@
 #define RT5651_D_GATE_EN_SFT			0
 
 /* Codec Private Register definition */
+
+/* MIC Over current threshold scale factor (0x15) */
+#define RT5651_MIC_OVCD_SF_MASK			(0x3 << 8)
+#define RT5651_MIC_OVCD_SF_SFT			8
+#define RT5651_MIC_OVCD_SF_0P5			(0x0 << 8)
+#define RT5651_MIC_OVCD_SF_0P75			(0x1 << 8)
+#define RT5651_MIC_OVCD_SF_1P0			(0x2 << 8)
+#define RT5651_MIC_OVCD_SF_1P5			(0x3 << 8)
+
 /* 3D Speaker Control (0x63) */
 #define RT5651_3D_SPK_MASK			(0x1 << 15)
 #define RT5651_3D_SPK_SFT			15
@@ -2065,6 +2075,7 @@ struct rt5651_priv {
 	struct delayed_work jack_detect_work;
 	enum rt5651_jd_src jd_src;
 	unsigned int ovcd_th;
+	unsigned int ovcd_sf;
 
 	int irq;
 	int sysclk;
-- 
2.16.2


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

[PATCH v3 06/22] ASoC: rt5651: Enable sticky mode for OVCD

[Hans de Goede]

When the mic-gnd contacts are short-circuited by a headphones plug, the
hardware periodically retries if it can apply the bias-current leading to
the OVCD status flip-flopping 1-0-1 with it being 0 about 10% of the time.
This commit enables the sticky bit for the OVCD status to deal with this.

This commit also introduces 2 helper functions to deal with the OVCD
status bit, this may seem a bit overkill now, but these will also be
used in future patches.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 4f78694adf70..3a439c121b0c 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1578,6 +1578,22 @@ static void rt5651_disable_micbias1_for_ovcd(struct snd_soc_component *component
 	snd_soc_dapm_mutex_unlock(dapm);
 }
 
+static void rt5651_clear_micbias1_ovcd(struct snd_soc_component *component)
+{
+	snd_soc_component_update_bits(component, RT5651_IRQ_CTRL2,
+		RT5651_MB1_OC_CLR, 0);
+}
+
+static bool rt5651_micbias1_ovcd(struct snd_soc_component *component)
+{
+	int val;
+
+	val = snd_soc_component_read32(component, RT5651_IRQ_CTRL2);
+	dev_dbg(component->dev, "irq ctrl2 %#04x\n", val);
+
+	return (val & RT5651_MB1_OC_CLR);
+}
+
 static irqreturn_t rt5651_irq(int irq, void *data)
 {
 	struct rt5651_priv *rt5651 = data;
@@ -1646,6 +1662,18 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 				      RT5651_PWR_MB_PU |
 				      RT5651_PWR_CLK12M_PU);
 
+	/*
+	 * The over-current-detect is only reliable in detecting the absence
+	 * of over-current, when the mic-contact in the jack is short-circuited,
+	 * the hardware periodically retries if it can apply the bias-current
+	 * leading to the ovcd status flip-flopping 1-0-1 with it being 0 about
+	 * 10% of the time, as we poll the ovcd status bit we might hit that
+	 * 10%, so we enable sticky mode and when checking OVCD we clear the
+	 * status, msleep() a bit and then check to get a reliable reading.
+	 */
+	snd_soc_component_update_bits(component, RT5651_IRQ_CTRL2,
+		RT5651_MB1_OC_STKY_MASK, RT5651_MB1_OC_STKY_EN);
+
 	rt5651->hp_jack = hp_jack;
 
 	ret = devm_request_threaded_irq(component->dev, rt5651->irq, NULL,
@@ -1879,13 +1907,12 @@ static int rt5651_jack_detect(struct snd_soc_component *component, int jack_inse
 
 	if (jack_insert) {
 		rt5651_enable_micbias1_for_ovcd(component);
+		rt5651_clear_micbias1_ovcd(component);
 		msleep(100);
-		if (snd_soc_component_read32(component, RT5651_IRQ_CTRL2) & RT5651_MB1_OC_CLR)
+		if (rt5651_micbias1_ovcd(component))
 			jack_type = SND_JACK_HEADPHONE;
 		else
 			jack_type = SND_JACK_HEADSET;
-		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL2,
-				    RT5651_MB1_OC_CLR, 0);
 		rt5651_disable_micbias1_for_ovcd(component);
 	} else { /* jack out */
 		jack_type = 0;
-- 
2.14.3

[PATCH v3 07/22] ASoC: rt5651: Enable Platform Clock during jack-type detect

[Hans de Goede]

When using RCCLK instead of MCLK / PLL1 the OVCD status often gets stuck
at its last value, which breaks jack-type detection.

This commit fixes this by force-enabling the platform clock when doing
jack-type detection.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 3a439c121b0c..9f522a0364f4 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1563,6 +1563,8 @@ static void rt5651_enable_micbias1_for_ovcd(struct snd_soc_component *component)
 	snd_soc_dapm_mutex_lock(dapm);
 	snd_soc_dapm_force_enable_pin_unlocked(dapm, "LDO");
 	snd_soc_dapm_force_enable_pin_unlocked(dapm, "micbias1");
+	/* OVCD is unreliable when used with RCCLK as sysclk-source */
+	snd_soc_dapm_force_enable_pin_unlocked(dapm, "Platform Clock");
 	snd_soc_dapm_sync_unlocked(dapm);
 	snd_soc_dapm_mutex_unlock(dapm);
 }
@@ -1572,6 +1574,7 @@ static void rt5651_disable_micbias1_for_ovcd(struct snd_soc_component *component
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 
 	snd_soc_dapm_mutex_lock(dapm);
+	snd_soc_dapm_disable_pin_unlocked(dapm, "Platform Clock");
 	snd_soc_dapm_disable_pin_unlocked(dapm, "micbias1");
 	snd_soc_dapm_disable_pin_unlocked(dapm, "LDO");
 	snd_soc_dapm_sync_unlocked(dapm);
-- 
2.14.3

[PATCH v3 08/22] ASoC: rt5651: Add rt5651_jack_inserted() helper

[Hans de Goede]

Add rt5651_jack_inserted() helper to get the jack-detect switch status,
This is a preparation patch for rewriting the jack type-detection to
make it more reliable.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 9f522a0364f4..3870c0f3b30f 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1597,6 +1597,31 @@ static bool rt5651_micbias1_ovcd(struct snd_soc_component *component)
 	return (val & RT5651_MB1_OC_CLR);
 }
 
+static bool rt5651_jack_inserted(struct snd_soc_component *component)
+{
+	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
+	int val;
+
+	val = snd_soc_component_read32(component, RT5651_INT_IRQ_ST);
+	dev_dbg(component->dev, "irq status %#04x\n", val);
+
+	switch (rt5651->jd_src) {
+	case RT5651_JD1_1:
+		val &= 0x1000;
+		break;
+	case RT5651_JD1_2:
+		val &= 0x2000;
+		break;
+	case RT5651_JD2:
+		val &= 0x4000;
+		break;
+	default:
+		break;
+	}
+
+	return val == 0;
+}
+
 static irqreturn_t rt5651_irq(int irq, void *data)
 {
 	struct rt5651_priv *rt5651 = data;
@@ -1928,27 +1953,13 @@ static void rt5651_jack_detect_work(struct work_struct *work)
 {
 	struct rt5651_priv *rt5651 =
 		container_of(work, struct rt5651_priv, jack_detect_work.work);
-
-	int report, val = 0;
+	int report, jack_inserted;
 
 	if (!rt5651->component)
 		return;
 
-	switch (rt5651->jd_src) {
-	case RT5651_JD1_1:
-		val = snd_soc_component_read32(rt5651->component, RT5651_INT_IRQ_ST) & 0x1000;
-		break;
-	case RT5651_JD1_2:
-		val = snd_soc_component_read32(rt5651->component, RT5651_INT_IRQ_ST) & 0x2000;
-		break;
-	case RT5651_JD2:
-		val = snd_soc_component_read32(rt5651->component, RT5651_INT_IRQ_ST) & 0x4000;
-		break;
-	default:
-		break;
-	}
-
-	report = rt5651_jack_detect(rt5651->component, !val);
+	jack_inserted = rt5651_jack_inserted(rt5651->component);
+	report = rt5651_jack_detect(rt5651->component, jack_inserted);
 
 	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
 }
-- 
2.14.3

[PATCH v3 09/22] ASoC: rt5651: Rewrite jack-type detection

[Hans de Goede]

We get the insertion event before the jack is fully inserted at which point
the second ring on a TRRS connector may short the 2nd ring and sleeve
contacts. Testing has shown that this short-circuit may happen as late
as 500ms after the insertion event, but it never lasts longer then 300ms.

This commit changes the detection algorithm to require 5 identical OVCD
values in a row at 100 ms intervals to fix the jack-type sometimes getting
mis-detected.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 110 +++++++++++++++++++++++++++++-----------------
 sound/soc/codecs/rt5651.h |   2 +-
 2 files changed, 70 insertions(+), 42 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 3870c0f3b30f..6b5669f3e85d 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1622,12 +1622,76 @@ static bool rt5651_jack_inserted(struct snd_soc_component *component)
 	return val == 0;
 }
 
+/* Jack detect timings */
+#define JACK_SETTLE_TIME	100 /* milli seconds */
+#define JACK_DETECT_COUNT	5
+#define JACK_DETECT_MAXCOUNT	20  /* Aprox. 2 seconds worth of tries */
+
+static int rt5651_detect_headset(struct snd_soc_component *component)
+{
+	int i, headset_count = 0, headphone_count = 0;
+
+	/*
+	 * We get the insertion event before the jack is fully inserted at which
+	 * point the second ring on a TRRS connector may short the 2nd ring and
+	 * sleeve contacts, also the overcurrent detection is not entirely
+	 * reliable. So we try several times with a wait in between until we
+	 * detect the same type JACK_DETECT_COUNT times in a row.
+	 */
+	for (i = 0; i < JACK_DETECT_MAXCOUNT; i++) {
+		/* Clear any previous over-current status flag */
+		rt5651_clear_micbias1_ovcd(component);
+
+		msleep(JACK_SETTLE_TIME);
+
+		/* Check the jack is still connected before checking ovcd */
+		if (!rt5651_jack_inserted(component))
+			return 0;
+
+		if (rt5651_micbias1_ovcd(component)) {
+			/*
+			 * Over current detected, there is a short between the
+			 * 2nd ring contact and the ground, so a TRS connector
+			 * without a mic contact and thus plain headphones.
+			 */
+			dev_dbg(component->dev, "mic-gnd shorted\n");
+			headset_count = 0;
+			headphone_count++;
+			if (headphone_count == JACK_DETECT_COUNT)
+				return SND_JACK_HEADPHONE;
+		} else {
+			dev_dbg(component->dev, "mic-gnd open\n");
+			headphone_count = 0;
+			headset_count++;
+			if (headset_count == JACK_DETECT_COUNT)
+				return SND_JACK_HEADSET;
+		}
+	}
+
+	dev_err(component->dev, "Error detecting headset vs headphones, bad contact?, assuming headphones\n");
+	return SND_JACK_HEADPHONE;
+}
+
+static void rt5651_jack_detect_work(struct work_struct *work)
+{
+	struct rt5651_priv *rt5651 =
+		container_of(work, struct rt5651_priv, jack_detect_work);
+	int report = 0;
+
+	if (rt5651_jack_inserted(rt5651->component)) {
+		rt5651_enable_micbias1_for_ovcd(rt5651->component);
+		report = rt5651_detect_headset(rt5651->component);
+		rt5651_disable_micbias1_for_ovcd(rt5651->component);
+	}
+
+	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
+}
+
 static irqreturn_t rt5651_irq(int irq, void *data)
 {
 	struct rt5651_priv *rt5651 = data;
 
-	queue_delayed_work(system_power_efficient_wq,
-			   &rt5651->jack_detect_work, msecs_to_jiffies(250));
+	queue_work(system_power_efficient_wq, &rt5651->jack_detect_work);
 
 	return IRQ_HANDLED;
 }
@@ -1715,8 +1779,7 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	}
 
 	/* sync initial jack state */
-	queue_delayed_work(system_power_efficient_wq,
-			   &rt5651->jack_detect_work, 0);
+	queue_work(system_power_efficient_wq, &rt5651->jack_detect_work);
 
 	return 0;
 }
@@ -1929,41 +1992,6 @@ static const struct i2c_device_id rt5651_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rt5651_i2c_id);
 
-static int rt5651_jack_detect(struct snd_soc_component *component, int jack_insert)
-{
-	int jack_type;
-
-	if (jack_insert) {
-		rt5651_enable_micbias1_for_ovcd(component);
-		rt5651_clear_micbias1_ovcd(component);
-		msleep(100);
-		if (rt5651_micbias1_ovcd(component))
-			jack_type = SND_JACK_HEADPHONE;
-		else
-			jack_type = SND_JACK_HEADSET;
-		rt5651_disable_micbias1_for_ovcd(component);
-	} else { /* jack out */
-		jack_type = 0;
-	}
-
-	return jack_type;
-}
-
-static void rt5651_jack_detect_work(struct work_struct *work)
-{
-	struct rt5651_priv *rt5651 =
-		container_of(work, struct rt5651_priv, jack_detect_work.work);
-	int report, jack_inserted;
-
-	if (!rt5651->component)
-		return;
-
-	jack_inserted = rt5651_jack_inserted(rt5651->component);
-	report = rt5651_jack_detect(rt5651->component, jack_inserted);
-
-	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
-}
-
 /*
  * Note this function MUST not look at device-properties, see the comment
  * above rt5651_apply_properties().
@@ -2006,7 +2034,7 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 	rt5651->irq = i2c->irq;
 	rt5651->hp_mute = 1;
 
-	INIT_DELAYED_WORK(&rt5651->jack_detect_work, rt5651_jack_detect_work);
+	INIT_WORK(&rt5651->jack_detect_work, rt5651_jack_detect_work);
 
 	ret = devm_snd_soc_register_component(&i2c->dev,
 				&soc_component_dev_rt5651,
@@ -2019,7 +2047,7 @@ static int rt5651_i2c_remove(struct i2c_client *i2c)
 {
 	struct rt5651_priv *rt5651 = i2c_get_clientdata(i2c);
 
-	cancel_delayed_work_sync(&rt5651->jack_detect_work);
+	cancel_work_sync(&rt5651->jack_detect_work);
 
 	return 0;
 }
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 71738ab93fb9..f20c9be94fb2 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2072,7 +2072,7 @@ struct rt5651_priv {
 	struct snd_soc_component *component;
 	struct regmap *regmap;
 	struct snd_soc_jack *hp_jack;
-	struct delayed_work jack_detect_work;
+	struct work_struct jack_detect_work;
 	enum rt5651_jd_src jd_src;
 	unsigned int ovcd_th;
 	unsigned int ovcd_sf;
-- 
2.14.3

Applied "ASoC: rt5651: Rewrite jack-type detection" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Rewrite jack-type detection

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 ee68096826fa16bca2f7bb555b3e0cbbe798d330 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:35:57 +0100
Subject: [PATCH] ASoC: rt5651: Rewrite jack-type detection

We get the insertion event before the jack is fully inserted at which point
the second ring on a TRRS connector may short the 2nd ring and sleeve
contacts. Testing has shown that this short-circuit may happen as late
as 500ms after the insertion event, but it never lasts longer then 300ms.

This commit changes the detection algorithm to require 5 identical OVCD
values in a row at 100 ms intervals to fix the jack-type sometimes getting
mis-detected.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 110 +++++++++++++++++++++++++++++-----------------
 sound/soc/codecs/rt5651.h |   2 +-
 2 files changed, 70 insertions(+), 42 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 41fc574dbc3d..3b9f5384fe7d 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1622,12 +1622,76 @@ static bool rt5651_jack_inserted(struct snd_soc_component *component)
 	return val == 0;
 }
 
+/* Jack detect timings */
+#define JACK_SETTLE_TIME	100 /* milli seconds */
+#define JACK_DETECT_COUNT	5
+#define JACK_DETECT_MAXCOUNT	20  /* Aprox. 2 seconds worth of tries */
+
+static int rt5651_detect_headset(struct snd_soc_component *component)
+{
+	int i, headset_count = 0, headphone_count = 0;
+
+	/*
+	 * We get the insertion event before the jack is fully inserted at which
+	 * point the second ring on a TRRS connector may short the 2nd ring and
+	 * sleeve contacts, also the overcurrent detection is not entirely
+	 * reliable. So we try several times with a wait in between until we
+	 * detect the same type JACK_DETECT_COUNT times in a row.
+	 */
+	for (i = 0; i < JACK_DETECT_MAXCOUNT; i++) {
+		/* Clear any previous over-current status flag */
+		rt5651_clear_micbias1_ovcd(component);
+
+		msleep(JACK_SETTLE_TIME);
+
+		/* Check the jack is still connected before checking ovcd */
+		if (!rt5651_jack_inserted(component))
+			return 0;
+
+		if (rt5651_micbias1_ovcd(component)) {
+			/*
+			 * Over current detected, there is a short between the
+			 * 2nd ring contact and the ground, so a TRS connector
+			 * without a mic contact and thus plain headphones.
+			 */
+			dev_dbg(component->dev, "mic-gnd shorted\n");
+			headset_count = 0;
+			headphone_count++;
+			if (headphone_count == JACK_DETECT_COUNT)
+				return SND_JACK_HEADPHONE;
+		} else {
+			dev_dbg(component->dev, "mic-gnd open\n");
+			headphone_count = 0;
+			headset_count++;
+			if (headset_count == JACK_DETECT_COUNT)
+				return SND_JACK_HEADSET;
+		}
+	}
+
+	dev_err(component->dev, "Error detecting headset vs headphones, bad contact?, assuming headphones\n");
+	return SND_JACK_HEADPHONE;
+}
+
+static void rt5651_jack_detect_work(struct work_struct *work)
+{
+	struct rt5651_priv *rt5651 =
+		container_of(work, struct rt5651_priv, jack_detect_work);
+	int report = 0;
+
+	if (rt5651_jack_inserted(rt5651->component)) {
+		rt5651_enable_micbias1_for_ovcd(rt5651->component);
+		report = rt5651_detect_headset(rt5651->component);
+		rt5651_disable_micbias1_for_ovcd(rt5651->component);
+	}
+
+	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
+}
+
 static irqreturn_t rt5651_irq(int irq, void *data)
 {
 	struct rt5651_priv *rt5651 = data;
 
-	queue_delayed_work(system_power_efficient_wq,
-			   &rt5651->jack_detect_work, msecs_to_jiffies(250));
+	queue_work(system_power_efficient_wq, &rt5651->jack_detect_work);
 
 	return IRQ_HANDLED;
 }
@@ -1715,8 +1779,7 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	}
 
 	/* sync initial jack state */
-	queue_delayed_work(system_power_efficient_wq,
-			   &rt5651->jack_detect_work, 0);
+	queue_work(system_power_efficient_wq, &rt5651->jack_detect_work);
 
 	return 0;
 }
@@ -1928,41 +1991,6 @@ static const struct i2c_device_id rt5651_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rt5651_i2c_id);
 
-static int rt5651_jack_detect(struct snd_soc_component *component, int jack_insert)
-{
-	int jack_type;
-
-	if (jack_insert) {
-		rt5651_enable_micbias1_for_ovcd(component);
-		rt5651_clear_micbias1_ovcd(component);
-		msleep(100);
-		if (rt5651_micbias1_ovcd(component))
-			jack_type = SND_JACK_HEADPHONE;
-		else
-			jack_type = SND_JACK_HEADSET;
-		rt5651_disable_micbias1_for_ovcd(component);
-	} else { /* jack out */
-		jack_type = 0;
-	}
-
-	return jack_type;
-}
-
-static void rt5651_jack_detect_work(struct work_struct *work)
-{
-	struct rt5651_priv *rt5651 =
-		container_of(work, struct rt5651_priv, jack_detect_work.work);
-	int report, jack_inserted;
-
-	if (!rt5651->component)
-		return;
-
-	jack_inserted = rt5651_jack_inserted(rt5651->component);
-	report = rt5651_jack_detect(rt5651->component, jack_inserted);
-
-	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
-}
-
 /*
  * Note this function MUST not look at device-properties, see the comment
  * above rt5651_apply_properties().
@@ -2005,7 +2033,7 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 	rt5651->irq = i2c->irq;
 	rt5651->hp_mute = 1;
 
-	INIT_DELAYED_WORK(&rt5651->jack_detect_work, rt5651_jack_detect_work);
+	INIT_WORK(&rt5651->jack_detect_work, rt5651_jack_detect_work);
 
 	ret = devm_snd_soc_register_component(&i2c->dev,
 				&soc_component_dev_rt5651,
@@ -2018,7 +2046,7 @@ static int rt5651_i2c_remove(struct i2c_client *i2c)
 {
 	struct rt5651_priv *rt5651 = i2c_get_clientdata(i2c);
 
-	cancel_delayed_work_sync(&rt5651->jack_detect_work);
+	cancel_work_sync(&rt5651->jack_detect_work);
 
 	return 0;
 }
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 71738ab93fb9..f20c9be94fb2 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2072,7 +2072,7 @@ struct rt5651_priv {
 	struct snd_soc_component *component;
 	struct regmap *regmap;
 	struct snd_soc_jack *hp_jack;
-	struct delayed_work jack_detect_work;
+	struct work_struct jack_detect_work;
 	enum rt5651_jd_src jd_src;
 	unsigned int ovcd_th;
 	unsigned int ovcd_sf;
-- 
2.16.2

[PATCH v3 10/22] ASoC: Intel: bytcr_rt5651: Not being able to find the codec ACPI-dev is an error

[Hans de Goede]

If we cannot find the codec ACPI-dev, then the snd-soc-core will not be
able to find the codec either and snd_soc_register_card() will just keep
exiting with -EPROBE_DEFER, filling the log with errors each time the
probe gets retried.

Instead simply log an error from the machine driver and exit with -ENODEV.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-This is a new patch in v3 of this patch-set
---
 sound/soc/intel/boards/bytcr_rt5651.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 8008a027cc93..6d8555e0359e 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -537,12 +537,13 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 
 	/* fixup codec name based on HID */
 	i2c_name = acpi_dev_get_first_match_name(mach->id, NULL, -1);
-	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;
+	if (!i2c_name) {
+		dev_err(&pdev->dev, "Error cannot find '%s' dev\n", mach->id);
+		return -ENODEV;
 	}
+	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);
-- 
2.14.3

Applied "ASoC: Intel: bytcr_rt5651: Not being able to find the codec ACPI-dev is an error" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Not being able to find the codec ACPI-dev is an error

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 e39cacc1b7de2a6d72ce49043c9cfd7dd129135a Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:35:58 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Not being able to find the codec
 ACPI-dev is an error

If we cannot find the codec ACPI-dev, then the snd-soc-core will not be
able to find the codec either and snd_soc_register_card() will just keep
exiting with -EPROBE_DEFER, filling the log with errors each time the
probe gets retried.

Instead simply log an error from the machine driver and exit with -ENODEV.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 456526a93dd5..105339c5b693 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -540,12 +540,13 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 
 	/* fixup codec name based on HID */
 	i2c_name = acpi_dev_get_first_match_name(mach->id, NULL, -1);
-	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;
+	if (!i2c_name) {
+		dev_err(&pdev->dev, "Error cannot find '%s' dev\n", mach->id);
+		return -ENODEV;
 	}
+	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);
-- 
2.16.2

[PATCH v3 11/22] ASoC: Intel: bytcr_rt5651: Pass jack-src info via device-properties

[Hans de Goede]

This commit add support for a new BYT_RT5651_JDSRC quirk, sets this
quirk for the KIANO SlimNote 14.2 laptop and uses the new "realtek,
jack-detect-source" property to pass this info to the codec driver.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-This has been split-out from the patch introducing the
 "realtek,jack-detect-source" property in the codec driver
-Apply properties before calling snd_soc_register_card, this avoids
 ordering issues and the need for a special rt5651_apply_properties() call
---
 sound/soc/intel/boards/bytcr_rt5651.c | 54 ++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 6d8555e0359e..abc44c9963bf 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -18,8 +18,10 @@
  */
 
 #include <linux/init.h>
+#include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/acpi.h>
 #include <linux/clk.h>
 #include <linux/device.h>
@@ -42,10 +44,21 @@ enum {
 	BYT_RT5651_IN3_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)
+enum {
+	BYT_RT5651_JD_NULL	= (RT5651_JD_NULL << 4),
+	BYT_RT5651_JD1_1	= (RT5651_JD1_1 << 4),
+	BYT_RT5651_JD1_2	= (RT5651_JD1_2 << 4),
+	BYT_RT5651_JD2		= (RT5651_JD2 << 4),
+};
+
+#define BYT_RT5651_MAP(quirk)		((quirk) & GENMASK(3, 0))
+#define BYT_RT5651_JDSRC(quirk)		(((quirk) & GENMASK(7, 4)) >> 4)
+#define BYT_RT5651_DMIC_EN		BIT(16)
+#define BYT_RT5651_MCLK_EN		BIT(17)
+#define BYT_RT5651_MCLK_25MHZ		BIT(18)
+
+/* jack-detect-source + terminating empty entry */
+#define MAX_NO_PROPS 2
 
 struct byt_rt5651_private {
 	struct clk *mclk;
@@ -66,6 +79,9 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN2_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN3_MAP)
 		dev_info(dev, "quirk IN3_MAP enabled");
+	if (BYT_RT5651_JDSRC(byt_rt5651_quirk))
+		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
+			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
 	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
 		dev_info(dev, "quirk DMIC enabled");
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN)
@@ -288,11 +304,35 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "KIANO SlimNote 14.2"),
 		},
 		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
 					BYT_RT5651_IN1_IN2_MAP),
 	},
 	{}
 };
 
+/*
+ * Note this MUST be called before snd_soc_register_card(), so that the props
+ * are in place before the codec component driver's probe function parses them.
+ */
+static int byt_rt5651_add_codec_device_props(const char *i2c_dev_name)
+{
+	struct property_entry props[MAX_NO_PROPS] = {};
+	struct device *i2c_dev;
+	int ret, cnt = 0;
+
+	i2c_dev = bus_find_device_by_name(&i2c_bus_type, NULL, i2c_dev_name);
+	if (!i2c_dev)
+		return -EPROBE_DEFER;
+
+	props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
+				BYT_RT5651_JDSRC(byt_rt5651_quirk));
+
+	ret = device_add_properties(i2c_dev, props);
+	put_device(i2c_dev);
+
+	return ret;
+}
+
 static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 {
 	struct snd_soc_card *card = runtime->card;
@@ -547,6 +587,12 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 
 	/* check quirks before creating card */
 	dmi_check_system(byt_rt5651_quirk_table);
+
+	/* Must be called before register_card, also see declaration comment. */
+	ret_val = byt_rt5651_add_codec_device_props(byt_rt5651_codec_name);
+	if (ret_val)
+		return ret_val;
+
 	log_quirks(&pdev->dev);
 
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) {
-- 
2.14.3

Applied "ASoC: Intel: bytcr_rt5651: Pass jack-src info via device-properties" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Pass jack-src info via device-properties

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 46058aeb6b10ab6620caa25984a3a4ee37d0fd56 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:35:59 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Pass jack-src info via
 device-properties

This commit add support for a new BYT_RT5651_JDSRC quirk, sets this
quirk for the KIANO SlimNote 14.2 laptop and uses the new "realtek,
jack-detect-source" property to pass this info to the codec driver.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 54 ++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 6288c27d87ce..499e405e591b 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -18,8 +18,10 @@
  */
 
 #include <linux/init.h>
+#include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/acpi.h>
 #include <linux/clk.h>
 #include <linux/device.h>
@@ -42,10 +44,21 @@ enum {
 	BYT_RT5651_IN3_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)
+enum {
+	BYT_RT5651_JD_NULL	= (RT5651_JD_NULL << 4),
+	BYT_RT5651_JD1_1	= (RT5651_JD1_1 << 4),
+	BYT_RT5651_JD1_2	= (RT5651_JD1_2 << 4),
+	BYT_RT5651_JD2		= (RT5651_JD2 << 4),
+};
+
+#define BYT_RT5651_MAP(quirk)		((quirk) & GENMASK(3, 0))
+#define BYT_RT5651_JDSRC(quirk)		(((quirk) & GENMASK(7, 4)) >> 4)
+#define BYT_RT5651_DMIC_EN		BIT(16)
+#define BYT_RT5651_MCLK_EN		BIT(17)
+#define BYT_RT5651_MCLK_25MHZ		BIT(18)
+
+/* jack-detect-source + terminating empty entry */
+#define MAX_NO_PROPS 2
 
 struct byt_rt5651_private {
 	struct clk *mclk;
@@ -66,6 +79,9 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN2_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN3_MAP)
 		dev_info(dev, "quirk IN3_MAP enabled");
+	if (BYT_RT5651_JDSRC(byt_rt5651_quirk))
+		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
+			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
 	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
 		dev_info(dev, "quirk DMIC enabled");
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN)
@@ -288,11 +304,35 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "KIANO SlimNote 14.2"),
 		},
 		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
 					BYT_RT5651_IN1_IN2_MAP),
 	},
 	{}
 };
 
+/*
+ * Note this MUST be called before snd_soc_register_card(), so that the props
+ * are in place before the codec component driver's probe function parses them.
+ */
+static int byt_rt5651_add_codec_device_props(const char *i2c_dev_name)
+{
+	struct property_entry props[MAX_NO_PROPS] = {};
+	struct device *i2c_dev;
+	int ret, cnt = 0;
+
+	i2c_dev = bus_find_device_by_name(&i2c_bus_type, NULL, i2c_dev_name);
+	if (!i2c_dev)
+		return -EPROBE_DEFER;
+
+	props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
+				BYT_RT5651_JDSRC(byt_rt5651_quirk));
+
+	ret = device_add_properties(i2c_dev, props);
+	put_device(i2c_dev);
+
+	return ret;
+}
+
 static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 {
 	struct snd_soc_card *card = runtime->card;
@@ -550,6 +590,12 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 
 	/* check quirks before creating card */
 	dmi_check_system(byt_rt5651_quirk_table);
+
+	/* Must be called before register_card, also see declaration comment. */
+	ret_val = byt_rt5651_add_codec_device_props(byt_rt5651_codec_name);
+	if (ret_val)
+		return ret_val;
+
 	log_quirks(&pdev->dev);
 
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) {
-- 
2.16.2

[PATCH v3 12/22] ASoC: Intel: bytcr_rt5651: Actually honor the DMIC_EN quirk if specified

[Hans de Goede]

Before this commit it was possible to set the DMIC_EN quirk in the machine
driver, but it would never be passed to the codec driver so it was a nop.

This commit adds code to actually pass the quirk to the codec driver.

Since the DMIC_EN quirk was ignored before, this commit removes it from
the default quirk settings, to avoid this causing an unexpected functional
change. If we really want the DMIC_EN behavior anywhere it should be
specifically enabled by follow up commits.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index abc44c9963bf..5cc2e6484df7 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -57,8 +57,8 @@ enum {
 #define BYT_RT5651_MCLK_EN		BIT(17)
 #define BYT_RT5651_MCLK_25MHZ		BIT(18)
 
-/* jack-detect-source + terminating empty entry */
-#define MAX_NO_PROPS 2
+/* jack-detect-source + dmic-en + terminating empty entry */
+#define MAX_NO_PROPS 3
 
 struct byt_rt5651_private {
 	struct clk *mclk;
@@ -66,7 +66,6 @@ struct byt_rt5651_private {
 };
 
 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)
@@ -327,6 +326,9 @@ static int byt_rt5651_add_codec_device_props(const char *i2c_dev_name)
 	props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
 				BYT_RT5651_JDSRC(byt_rt5651_quirk));
 
+	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
+		props[cnt++] = PROPERTY_ENTRY_BOOL("realtek,dmic-en");
+
 	ret = device_add_properties(i2c_dev, props);
 	put_device(i2c_dev);
 
-- 
2.14.3

Applied "ASoC: Intel: bytcr_rt5651: Actually honor the DMIC_EN quirk if specified" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Actually honor the DMIC_EN quirk if specified

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 c2f26938d2a456dcf429385617e58cfd510a64a8 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:36:00 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Actually honor the DMIC_EN quirk
 if specified

Before this commit it was possible to set the DMIC_EN quirk in the machine
driver, but it would never be passed to the codec driver so it was a nop.

This commit adds code to actually pass the quirk to the codec driver.

Since the DMIC_EN quirk was ignored before, this commit removes it from
the default quirk settings, to avoid this causing an unexpected functional
change. If we really want the DMIC_EN behavior anywhere it should be
specifically enabled by follow up commits.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 499e405e591b..a874bba3aec0 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -57,8 +57,8 @@ enum {
 #define BYT_RT5651_MCLK_EN		BIT(17)
 #define BYT_RT5651_MCLK_25MHZ		BIT(18)
 
-/* jack-detect-source + terminating empty entry */
-#define MAX_NO_PROPS 2
+/* jack-detect-source + dmic-en + terminating empty entry */
+#define MAX_NO_PROPS 3
 
 struct byt_rt5651_private {
 	struct clk *mclk;
@@ -66,7 +66,6 @@ struct byt_rt5651_private {
 };
 
 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)
@@ -327,6 +326,9 @@ static int byt_rt5651_add_codec_device_props(const char *i2c_dev_name)
 	props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
 				BYT_RT5651_JDSRC(byt_rt5651_quirk));
 
+	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
+		props[cnt++] = PROPERTY_ENTRY_BOOL("realtek,dmic-en");
+
 	ret = device_add_properties(i2c_dev, props);
 	put_device(i2c_dev);
 
-- 
2.16.2

[PATCH v3 13/22] ASoC: Intel: bytcr_rt5651: Only create jack if we have a jack-detect source

[Hans de Goede]

Only create the jack if we have a valid jack-detect source and properly
check the snd_soc_component_set_jack() return value.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 5cc2e6484df7..3f71c018aa8e 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -402,17 +402,21 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 			dev_err(card->dev, "unable to set MCLK rate\n");
 	}
 
-	ret = snd_soc_card_jack_new(runtime->card, "Headset",
+	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
+		ret = snd_soc_card_jack_new(runtime->card, "Headset",
 				    SND_JACK_HEADSET, &priv->jack,
 				    bytcr_jack_pins, ARRAY_SIZE(bytcr_jack_pins));
-	if (ret) {
-		dev_err(runtime->dev, "Headset jack creation failed %d\n", ret);
-		return ret;
-	}
+		if (ret) {
+			dev_err(runtime->dev, "jack creation failed %d\n", ret);
+			return ret;
+		}
 
-	snd_soc_component_set_jack(codec, &priv->jack, NULL);
+		ret = snd_soc_component_set_jack(codec, &priv->jack, NULL);
+		if (ret)
+			return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 static const struct snd_soc_pcm_stream byt_rt5651_dai_params = {
-- 
2.14.3

Applied "ASoC: Intel: bytcr_rt5651: Only create jack if we have a jack-detect source" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Only create jack if we have a jack-detect source

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 aed859a2c7ac8c44ba9b3a6f82b5f08da8a0a975 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:36:01 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Only create jack if we have a
 jack-detect source

Only create the jack if we have a valid jack-detect source and properly
check the snd_soc_component_set_jack() return value.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index a874bba3aec0..56f1f076d92c 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -404,17 +404,21 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 			dev_err(card->dev, "unable to set MCLK rate\n");
 	}
 
-	ret = snd_soc_card_jack_new(runtime->card, "Headset",
+	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
+		ret = snd_soc_card_jack_new(runtime->card, "Headset",
 				    SND_JACK_HEADSET, &priv->jack,
 				    bytcr_jack_pins, ARRAY_SIZE(bytcr_jack_pins));
-	if (ret) {
-		dev_err(runtime->dev, "Headset jack creation failed %d\n", ret);
-		return ret;
-	}
+		if (ret) {
+			dev_err(runtime->dev, "jack creation failed %d\n", ret);
+			return ret;
+		}
 
-	snd_soc_component_set_jack(codec, &priv->jack, NULL);
+		ret = snd_soc_component_set_jack(codec, &priv->jack, NULL);
+		if (ret)
+			return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 static const struct snd_soc_pcm_stream byt_rt5651_dai_params = {
-- 
2.16.2

[PATCH v3 14/22] ASoC: Intel: bytcr_rt5651: Add quirk micbias OVCD configuration

[Hans de Goede]

Add support for setting the micbias OVCD limits device-properties through
quirks.

And set the limits for this to 2000uA with a scale-factor of 0.75 for the
KIANO SlimNote 14.2 device, which is the only device on which
jack-detection is currently enabled.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 3f71c018aa8e..c7ca423f025c 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -51,14 +51,29 @@ enum {
 	BYT_RT5651_JD2		= (RT5651_JD2 << 4),
 };
 
+enum {
+	BYT_RT5651_OVCD_TH_600UA  = (6 << 8),
+	BYT_RT5651_OVCD_TH_1500UA = (15 << 8),
+	BYT_RT5651_OVCD_TH_2000UA = (20 << 8),
+};
+
+enum {
+	BYT_RT5651_OVCD_SF_0P5	= (RT5651_OVCD_SF_0P5 << 13),
+	BYT_RT5651_OVCD_SF_0P75	= (RT5651_OVCD_SF_0P75 << 13),
+	BYT_RT5651_OVCD_SF_1P0	= (RT5651_OVCD_SF_1P0 << 13),
+	BYT_RT5651_OVCD_SF_1P5	= (RT5651_OVCD_SF_1P5 << 13),
+};
+
 #define BYT_RT5651_MAP(quirk)		((quirk) & GENMASK(3, 0))
 #define BYT_RT5651_JDSRC(quirk)		(((quirk) & GENMASK(7, 4)) >> 4)
+#define BYT_RT5651_OVCD_TH(quirk)	(((quirk) & GENMASK(12, 8)) >> 8)
+#define BYT_RT5651_OVCD_SF(quirk)	(((quirk) & GENMASK(14, 13)) >> 13)
 #define BYT_RT5651_DMIC_EN		BIT(16)
 #define BYT_RT5651_MCLK_EN		BIT(17)
 #define BYT_RT5651_MCLK_25MHZ		BIT(18)
 
-/* jack-detect-source + dmic-en + terminating empty entry */
-#define MAX_NO_PROPS 3
+/* jack-detect-source + dmic-en + ovcd-th + -sf + terminating empty entry */
+#define MAX_NO_PROPS 5
 
 struct byt_rt5651_private {
 	struct clk *mclk;
@@ -78,9 +93,14 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN2_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN3_MAP)
 		dev_info(dev, "quirk IN3_MAP enabled");
-	if (BYT_RT5651_JDSRC(byt_rt5651_quirk))
+	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
 		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
 			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
+		dev_info(dev, "quirk realtek,over-current-threshold-microamp %ld\n",
+			 BYT_RT5651_OVCD_TH(byt_rt5651_quirk) * 100);
+		dev_info(dev, "quirk realtek,over-current-scale-factor %ld\n",
+			 BYT_RT5651_OVCD_SF(byt_rt5651_quirk));
+	}
 	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
 		dev_info(dev, "quirk DMIC enabled");
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN)
@@ -304,6 +324,8 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 		},
 		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
 					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_0P75 |
 					BYT_RT5651_IN1_IN2_MAP),
 	},
 	{}
@@ -326,6 +348,12 @@ static int byt_rt5651_add_codec_device_props(const char *i2c_dev_name)
 	props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
 				BYT_RT5651_JDSRC(byt_rt5651_quirk));
 
+	props[cnt++] = PROPERTY_ENTRY_U32("realtek,over-current-threshold-microamp",
+				BYT_RT5651_OVCD_TH(byt_rt5651_quirk) * 100);
+
+	props[cnt++] = PROPERTY_ENTRY_U32("realtek,over-current-scale-factor",
+				BYT_RT5651_OVCD_SF(byt_rt5651_quirk));
+
 	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
 		props[cnt++] = PROPERTY_ENTRY_BOOL("realtek,dmic-en");
 
-- 
2.14.3

Applied "ASoC: Intel: bytcr_rt5651: Add quirk micbias OVCD configuration" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Add quirk micbias OVCD configuration

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 8ffaa6a136e6f30e053e78fb9742c7748bef8576 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:36:02 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Add quirk micbias OVCD
 configuration

Add support for setting the micbias OVCD limits device-properties through
quirks.

And set the limits for this to 2000uA with a scale-factor of 0.75 for the
KIANO SlimNote 14.2 device, which is the only device on which
jack-detection is currently enabled.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 56f1f076d92c..91b5841af622 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -51,14 +51,29 @@ enum {
 	BYT_RT5651_JD2		= (RT5651_JD2 << 4),
 };
 
+enum {
+	BYT_RT5651_OVCD_TH_600UA  = (6 << 8),
+	BYT_RT5651_OVCD_TH_1500UA = (15 << 8),
+	BYT_RT5651_OVCD_TH_2000UA = (20 << 8),
+};
+
+enum {
+	BYT_RT5651_OVCD_SF_0P5	= (RT5651_OVCD_SF_0P5 << 13),
+	BYT_RT5651_OVCD_SF_0P75	= (RT5651_OVCD_SF_0P75 << 13),
+	BYT_RT5651_OVCD_SF_1P0	= (RT5651_OVCD_SF_1P0 << 13),
+	BYT_RT5651_OVCD_SF_1P5	= (RT5651_OVCD_SF_1P5 << 13),
+};
+
 #define BYT_RT5651_MAP(quirk)		((quirk) & GENMASK(3, 0))
 #define BYT_RT5651_JDSRC(quirk)		(((quirk) & GENMASK(7, 4)) >> 4)
+#define BYT_RT5651_OVCD_TH(quirk)	(((quirk) & GENMASK(12, 8)) >> 8)
+#define BYT_RT5651_OVCD_SF(quirk)	(((quirk) & GENMASK(14, 13)) >> 13)
 #define BYT_RT5651_DMIC_EN		BIT(16)
 #define BYT_RT5651_MCLK_EN		BIT(17)
 #define BYT_RT5651_MCLK_25MHZ		BIT(18)
 
-/* jack-detect-source + dmic-en + terminating empty entry */
-#define MAX_NO_PROPS 3
+/* jack-detect-source + dmic-en + ovcd-th + -sf + terminating empty entry */
+#define MAX_NO_PROPS 5
 
 struct byt_rt5651_private {
 	struct clk *mclk;
@@ -78,9 +93,14 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN2_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN3_MAP)
 		dev_info(dev, "quirk IN3_MAP enabled");
-	if (BYT_RT5651_JDSRC(byt_rt5651_quirk))
+	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
 		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
 			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
+		dev_info(dev, "quirk realtek,over-current-threshold-microamp %ld\n",
+			 BYT_RT5651_OVCD_TH(byt_rt5651_quirk) * 100);
+		dev_info(dev, "quirk realtek,over-current-scale-factor %ld\n",
+			 BYT_RT5651_OVCD_SF(byt_rt5651_quirk));
+	}
 	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
 		dev_info(dev, "quirk DMIC enabled");
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN)
@@ -304,6 +324,8 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 		},
 		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
 					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_0P75 |
 					BYT_RT5651_IN1_IN2_MAP),
 	},
 	{}
@@ -326,6 +348,12 @@ static int byt_rt5651_add_codec_device_props(const char *i2c_dev_name)
 	props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
 				BYT_RT5651_JDSRC(byt_rt5651_quirk));
 
+	props[cnt++] = PROPERTY_ENTRY_U32("realtek,over-current-threshold-microamp",
+				BYT_RT5651_OVCD_TH(byt_rt5651_quirk) * 100);
+
+	props[cnt++] = PROPERTY_ENTRY_U32("realtek,over-current-scale-factor",
+				BYT_RT5651_OVCD_SF(byt_rt5651_quirk));
+
 	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
 		props[cnt++] = PROPERTY_ENTRY_BOOL("realtek,dmic-en");
 
-- 
2.16.2

[PATCH v3 15/22] ASoC: Intel: bytcr_rt5651: Configure PLL1 before using it

[Hans de Goede]

When platform_clock_control() first selects PLL1 as sysclk the PLL_CTRL
registers have not been setup yet and we effectively have an invalid clock
configuration until byt_rt5651_aif1_hw_params() gets called.

Add a new byt_rt5651_prepare_and_enable_pll1() helper and use that from
both platform_clock_control() and byt_rt5651_aif1_hw_params() to fix this.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Make bclk_ratio configurable instead of hardcoding it to 50, this will be
 used when adding SSP0 support
---
 sound/soc/intel/boards/bytcr_rt5651.c | 73 +++++++++++++++++------------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index c7ca423f025c..987051161c9f 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -111,6 +111,38 @@ static void log_quirks(struct device *dev)
 
 #define BYT_CODEC_DAI1	"rt5651-aif1"
 
+static int byt_rt5651_prepare_and_enable_pll1(struct snd_soc_dai *codec_dai,
+					      int rate, int bclk_ratio)
+{
+	int clk_id, clk_freq, ret;
+
+	/* Configure the PLL before selecting it */
+	if (!(byt_rt5651_quirk & BYT_RT5651_MCLK_EN)) {
+		clk_id = RT5651_PLL1_S_BCLK1,
+		clk_freq = rate * bclk_ratio;
+	} else {
+		clk_id = RT5651_PLL1_S_MCLK;
+		if (byt_rt5651_quirk & BYT_RT5651_MCLK_25MHZ)
+			clk_freq = 25000000;
+		else
+			clk_freq = 19200000;
+	}
+	ret = snd_soc_dai_set_pll(codec_dai, 0, clk_id, clk_freq, rate * 512);
+	if (ret < 0) {
+		dev_err(codec_dai->codec->dev, "can't set pll: %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_dai_set_sysclk(codec_dai, RT5651_SCLK_S_PLL1,
+				     rate * 512, SND_SOC_CLOCK_IN);
+	if (ret < 0) {
+		dev_err(codec_dai->codec->dev, "can't set clock %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int platform_clock_control(struct snd_soc_dapm_widget *w,
 				  struct snd_kcontrol *k, int  event)
 {
@@ -136,9 +168,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 				return ret;
 			}
 		}
-		ret = snd_soc_dai_set_sysclk(codec_dai, RT5651_SCLK_S_PLL1,
-					     48000 * 512,
-					     SND_SOC_CLOCK_IN);
+		ret = byt_rt5651_prepare_and_enable_pll1(codec_dai, 48000, 50);
 	} else {
 		/*
 		 * Set codec clock source to internal clock before
@@ -252,44 +282,11 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
-	int ret;
+	int rate = params_rate(params);
 
 	snd_soc_dai_set_bclk_ratio(codec_dai, 50);
 
-	ret = snd_soc_dai_set_sysclk(codec_dai, RT5651_SCLK_S_PLL1,
-				     params_rate(params) * 512,
-				     SND_SOC_CLOCK_IN);
-	if (ret < 0) {
-		dev_err(rtd->dev, "can't set codec clock %d\n", ret);
-		return ret;
-	}
-
-	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;
-	}
-
-	return 0;
+	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
 }
 
 static int byt_rt5651_quirk_cb(const struct dmi_system_id *id)
-- 
2.14.3

[PATCH v3 16/22] ASoC: Intel: bytcr_rt5651: Drop snd_soc_dai_set_bclk_ratio() call

[Hans de Goede]

Drop the snd_soc_dai_set_bclk_ratio() call, the rt5651 dai does not have a
set_bclk_ratio() op, so it is a nop (and returns -EINVAL).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 987051161c9f..fa4184476158 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -284,8 +284,6 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	int rate = params_rate(params);
 
-	snd_soc_dai_set_bclk_ratio(codec_dai, 50);
-
 	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
 }
 
-- 
2.14.3

[PATCH v3 17/22] ASoC: Intel: bytcr_rt5651: Rename IN3_MAP to IN1_HS_IN3_MAP

[Hans de Goede]

All the mappings are named for where the internal mic is routed and in that
sense the newly added in3_map really is the same as in1_map, what makes it
different is that it maps the headset mic at IN3 rather then at IN2.

Rename in3_map to in1_hs_in3_map to better reflect what it actually does.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index fa4184476158..61af8904d5f4 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -41,7 +41,7 @@ enum {
 	BYT_RT5651_IN1_MAP,
 	BYT_RT5651_IN2_MAP,
 	BYT_RT5651_IN1_IN2_MAP,
-	BYT_RT5651_IN3_MAP,
+	BYT_RT5651_IN1_HS_IN3_MAP,
 };
 
 enum {
@@ -91,8 +91,8 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN1_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN2_MAP)
 		dev_info(dev, "quirk IN2_MAP enabled");
-	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN3_MAP)
-		dev_info(dev, "quirk IN3_MAP enabled");
+	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN1_HS_IN3_MAP)
+		dev_info(dev, "quirk IN1_HS_IN3_MAP enabled");
 	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
 		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
 			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
@@ -235,8 +235,8 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_dmic_map[] = {
 
 static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_map[] = {
 	{"Internal Mic", NULL, "micbias1"},
-	{"IN2P", NULL, "Headset Mic"},
 	{"IN1P", NULL, "Internal Mic"},
+	{"IN2P", NULL, "Headset Mic"},
 };
 
 static const struct snd_soc_dapm_route byt_rt5651_intmic_in2_map[] = {
@@ -252,10 +252,10 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_in2_map[] = {
 	{"IN3P", NULL, "Headset Mic"},
 };
 
-static const struct snd_soc_dapm_route byt_rt5651_intmic_in3_map[] = {
+static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_hs_in3_map[] = {
 	{"Internal Mic", NULL, "micbias1"},
-	{"IN3P", NULL, "Headset Mic"},
 	{"IN1P", NULL, "Internal Mic"},
+	{"IN3P", NULL, "Headset Mic"},
 };
 
 static const struct snd_kcontrol_new byt_rt5651_controls[] = {
@@ -300,7 +300,7 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Circuitco"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "Minnowboard Max B3 PLATFORM"),
 		},
-		.driver_data = (void *)(BYT_RT5651_IN3_MAP),
+		.driver_data = (void *)(BYT_RT5651_IN1_HS_IN3_MAP),
 	},
 	{
 		.callback = byt_rt5651_quirk_cb,
@@ -309,7 +309,7 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Minnowboard Turbot"),
 		},
 		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
-					BYT_RT5651_IN3_MAP),
+					BYT_RT5651_IN1_HS_IN3_MAP),
 	},
 	{
 		.callback = byt_rt5651_quirk_cb,
@@ -382,9 +382,9 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 		custom_map = byt_rt5651_intmic_in1_in2_map;
 		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in1_in2_map);
 		break;
-	case BYT_RT5651_IN3_MAP:
-		custom_map = byt_rt5651_intmic_in3_map;
-		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in3_map);
+	case BYT_RT5651_IN1_HS_IN3_MAP:
+		custom_map = byt_rt5651_intmic_in1_hs_in3_map;
+		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in1_hs_in3_map);
 		break;
 	default:
 		custom_map = byt_rt5651_intmic_dmic_map;
-- 
2.14.3

[PATCH v3 18/22] ASoC: Intel: bytcr_rt5651: Add new IN2_HS_IN3 input map and a quirk using it

[Hans de Goede]

Add a new IN2_HS_IN3 input map and add a quirk for the input mapping and
jack-detect source for the Chuwi Vi8 Plus tablet, which uses this new map.

Note the Chuwi Vi8 Plus lists an extra GPIO in its codecs ACPI resources
which needs to be driven high to enable the external speaker amplifier,
this is not supported yet and will be fixed in a future patch.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 61af8904d5f4..2a5a817e6fb4 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -42,6 +42,7 @@ enum {
 	BYT_RT5651_IN2_MAP,
 	BYT_RT5651_IN1_IN2_MAP,
 	BYT_RT5651_IN1_HS_IN3_MAP,
+	BYT_RT5651_IN2_HS_IN3_MAP,
 };
 
 enum {
@@ -93,6 +94,8 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN2_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN1_HS_IN3_MAP)
 		dev_info(dev, "quirk IN1_HS_IN3_MAP enabled");
+	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN2_HS_IN3_MAP)
+		dev_info(dev, "quirk IN2_HS_IN3_MAP enabled");
 	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
 		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
 			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
@@ -258,6 +261,12 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_hs_in3_map[] = {
 	{"IN3P", NULL, "Headset Mic"},
 };
 
+static const struct snd_soc_dapm_route byt_rt5651_intmic_in2_hs_in3_map[] = {
+	{"Internal Mic", NULL, "micbias1"},
+	{"IN2P", NULL, "Internal Mic"},
+	{"IN3P", NULL, "Headset Mic"},
+};
+
 static const struct snd_kcontrol_new byt_rt5651_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Headphone"),
 	SOC_DAPM_PIN_SWITCH("Headset Mic"),
@@ -323,6 +332,19 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 					BYT_RT5651_OVCD_SF_0P75 |
 					BYT_RT5651_IN1_IN2_MAP),
 	},
+	{
+		/* Chuwi Vi8 Plus (CWI519) */
+		.callback = byt_rt5651_quirk_cb,
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Hampoo"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "D2D3_Vi8A1"),
+		},
+		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_0P75 |
+					BYT_RT5651_IN2_HS_IN3_MAP),
+	},
 	{}
 };
 
@@ -386,6 +408,10 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 		custom_map = byt_rt5651_intmic_in1_hs_in3_map;
 		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in1_hs_in3_map);
 		break;
+	case BYT_RT5651_IN2_HS_IN3_MAP:
+		custom_map = byt_rt5651_intmic_in2_hs_in3_map;
+		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in2_hs_in3_map);
+		break;
 	default:
 		custom_map = byt_rt5651_intmic_dmic_map;
 		num_routes = ARRAY_SIZE(byt_rt5651_intmic_dmic_map);
-- 
2.14.3

[PATCH v3 19/22] ASoC: Intel: bytcr_rt5651: Add support for Bay Trail CR / SSP0 using boards

[Hans de Goede]

Despite its name being prefixed with bytcr, before this commit the
bytcr_rt5651 machine driver could not work with Bay Trail CR boards,
as those only have SSP0 and it only supported SSP0-AIF1 setups.

This commit adds support for this, autodetecting AIF1 vs AIF2 based on
BIOS tables.

While at it also add support for SSP2-AIF2 setups, as that requires only
minimal extra code on top of the code adding SSP0-AIF1 / SSP0-AIF2 support.

Note this code is all copy-pasted from bytcr_rt5640.c. I've looked into
merging the 2 machine drivers into 1 to avoid copy-pasting, but there are
enough subtile differences to make this hard *and* with all the quirks the
machine driver already is full with if (variant-foo) then ... else ...
constructs adding more of these is going to make the code unreadable.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Determine bclk_ratio to pass to byt_rt5651_prepare_and_enable_pll1()
 based on format in hw_params rather then on quirks, so that we only
 have to care about the ssp0 quirk in the fixup function and no in other
 places
---
 sound/soc/intel/boards/bytcr_rt5651.c | 217 +++++++++++++++++++++++++++++++---
 1 file changed, 201 insertions(+), 16 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 2a5a817e6fb4..1f48b2920f48 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -27,6 +27,7 @@
 #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 <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -72,6 +73,9 @@ enum {
 #define BYT_RT5651_DMIC_EN		BIT(16)
 #define BYT_RT5651_MCLK_EN		BIT(17)
 #define BYT_RT5651_MCLK_25MHZ		BIT(18)
+#define BYT_RT5651_SSP2_AIF2		BIT(19) /* default is using AIF1  */
+#define BYT_RT5651_SSP0_AIF1		BIT(20)
+#define BYT_RT5651_SSP0_AIF2		BIT(21)
 
 /* jack-detect-source + dmic-en + ovcd-th + -sf + terminating empty entry */
 #define MAX_NO_PROPS 5
@@ -81,8 +85,7 @@ struct byt_rt5651_private {
 	struct snd_soc_jack jack;
 };
 
-static unsigned long byt_rt5651_quirk = BYT_RT5651_DMIC_MAP |
-					BYT_RT5651_MCLK_EN;
+static unsigned long byt_rt5651_quirk = BYT_RT5651_MCLK_EN;
 
 static void log_quirks(struct device *dev)
 {
@@ -110,9 +113,16 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk MCLK_EN enabled");
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_25MHZ)
 		dev_info(dev, "quirk MCLK_25MHZ enabled");
+	if (byt_rt5651_quirk & BYT_RT5651_SSP2_AIF2)
+		dev_info(dev, "quirk SSP2_AIF2 enabled\n");
+	if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1)
+		dev_info(dev, "quirk SSP0_AIF1 enabled\n");
+	if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)
+		dev_info(dev, "quirk SSP0_AIF2 enabled\n");
 }
 
 #define BYT_CODEC_DAI1	"rt5651-aif1"
+#define BYT_CODEC_DAI2	"rt5651-aif2"
 
 static int byt_rt5651_prepare_and_enable_pll1(struct snd_soc_dai *codec_dai,
 					      int rate, int bclk_ratio)
@@ -156,6 +166,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 	int ret;
 
 	codec_dai = snd_soc_card_get_codec_dai(card, BYT_CODEC_DAI1);
+	if (!codec_dai)
+		codec_dai = snd_soc_card_get_codec_dai(card, BYT_CODEC_DAI2);
 	if (!codec_dai) {
 		dev_err(card->dev,
 			"Codec dai not found; Unable to set platform clock\n");
@@ -213,13 +225,6 @@ static const struct snd_soc_dapm_route byt_rt5651_audio_map[] = {
 	{"Speaker", NULL, "Platform Clock"},
 	{"Line In", NULL, "Platform Clock"},
 
-	{"AIF1 Playback", NULL, "ssp2 Tx"},
-	{"ssp2 Tx", NULL, "codec_out0"},
-	{"ssp2 Tx", NULL, "codec_out1"},
-	{"codec_in0", NULL, "ssp2 Rx"},
-	{"codec_in1", NULL, "ssp2 Rx"},
-	{"ssp2 Rx", NULL, "AIF1 Capture"},
-
 	{"Headset Mic", NULL, "micbias1"}, /* lowercase for rt5651 */
 	{"Headphone", NULL, "HPOL"},
 	{"Headphone", NULL, "HPOR"},
@@ -267,6 +272,42 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_in2_hs_in3_map[] = {
 	{"IN3P", NULL, "Headset Mic"},
 };
 
+static const struct snd_soc_dapm_route byt_rt5651_ssp0_aif1_map[] = {
+	{"ssp0 Tx", NULL, "modem_out"},
+	{"modem_in", NULL, "ssp0 Rx"},
+
+	{"AIF1 Playback", NULL, "ssp0 Tx"},
+	{"ssp0 Rx", NULL, "AIF1 Capture"},
+};
+
+static const struct snd_soc_dapm_route byt_rt5651_ssp0_aif2_map[] = {
+	{"ssp0 Tx", NULL, "modem_out"},
+	{"modem_in", NULL, "ssp0 Rx"},
+
+	{"AIF2 Playback", NULL, "ssp0 Tx"},
+	{"ssp0 Rx", NULL, "AIF2 Capture"},
+};
+
+static const struct snd_soc_dapm_route byt_rt5651_ssp2_aif1_map[] = {
+	{"ssp2 Tx", NULL, "codec_out0"},
+	{"ssp2 Tx", NULL, "codec_out1"},
+	{"codec_in0", NULL, "ssp2 Rx"},
+	{"codec_in1", NULL, "ssp2 Rx"},
+
+	{"AIF1 Playback", NULL, "ssp2 Tx"},
+	{"ssp2 Rx", NULL, "AIF1 Capture"},
+};
+
+static const struct snd_soc_dapm_route byt_rt5651_ssp2_aif2_map[] = {
+	{"ssp2 Tx", NULL, "codec_out0"},
+	{"ssp2 Tx", NULL, "codec_out1"},
+	{"codec_in0", NULL, "ssp2 Rx"},
+	{"codec_in1", NULL, "ssp2 Rx"},
+
+	{"AIF2 Playback", NULL, "ssp2 Tx"},
+	{"ssp2 Rx", NULL, "AIF2 Capture"},
+};
+
 static const struct snd_kcontrol_new byt_rt5651_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Headphone"),
 	SOC_DAPM_PIN_SWITCH("Headset Mic"),
@@ -291,9 +332,16 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	snd_pcm_format_t format = params_format(params);
 	int rate = params_rate(params);
+	int bclk_ratio;
 
-	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
+	if (format == SNDRV_PCM_FORMAT_S16_LE)
+		bclk_ratio = 32;
+	else
+		bclk_ratio = 50;
+
+	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, bclk_ratio);
 }
 
 static int byt_rt5651_quirk_cb(const struct dmi_system_id *id)
@@ -420,6 +468,26 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 	if (ret)
 		return ret;
 
+	if (byt_rt5651_quirk & BYT_RT5651_SSP2_AIF2) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp2_aif2_map,
+					ARRAY_SIZE(byt_rt5651_ssp2_aif2_map));
+	} else if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp0_aif1_map,
+					ARRAY_SIZE(byt_rt5651_ssp0_aif1_map));
+	} else if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp0_aif2_map,
+					ARRAY_SIZE(byt_rt5651_ssp0_aif2_map));
+	} else {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp2_aif1_map,
+					ARRAY_SIZE(byt_rt5651_ssp2_aif1_map));
+	}
+	if (ret)
+		return ret;
+
 	ret = snd_soc_add_card_controls(card, byt_rt5651_controls,
 					ARRAY_SIZE(byt_rt5651_controls));
 	if (ret) {
@@ -483,18 +551,26 @@ static int byt_rt5651_codec_fixup(struct snd_soc_pcm_runtime *rtd,
 			SNDRV_PCM_HW_PARAM_RATE);
 	struct snd_interval *channels = hw_param_interval(params,
 						SNDRV_PCM_HW_PARAM_CHANNELS);
-	int ret;
+	int ret, bits;
 
-	/* The DSP will covert the FE rate to 48k, stereo, 24bits */
+	/* The DSP will covert the FE rate to 48k, stereo */
 	rate->min = rate->max = 48000;
 	channels->min = channels->max = 2;
 
-	/* set SSP2 to 24-bit */
-	params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
+	if ((byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1) ||
+	    (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) {
+		/* set SSP0 to 16-bit */
+		params_set_format(params, SNDRV_PCM_FORMAT_S16_LE);
+		bits = 16;
+	} else {
+		/* set SSP2 to 24-bit */
+		params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
+		bits = 24;
+	}
 
 	/*
 	 * Default mode for SSP configuration is TDM 4 slot, override config
-	 * with explicit setting to I2S 2ch 24-bit. The word length is set with
+	 * with explicit setting to I2S 2ch. The word length is set with
 	 * dai_set_tdm_slot() since there is no other API exposed
 	 */
 	ret = snd_soc_dai_set_fmt(rtd->cpu_dai,
@@ -508,7 +584,7 @@ static int byt_rt5651_codec_fixup(struct snd_soc_pcm_runtime *rtd,
 		return ret;
 	}
 
-	ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, 24);
+	ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, bits);
 	if (ret < 0) {
 		dev_err(rtd->dev, "can't set I2S config, err %d\n", ret);
 		return ret;
@@ -602,12 +678,32 @@ static struct snd_soc_card byt_rt5651_card = {
 };
 
 static char byt_rt5651_codec_name[SND_ACPI_I2C_ID_LEN];
+static char byt_rt5651_codec_aif_name[12]; /*  = "rt5651-aif[1|2]" */
+static char byt_rt5651_cpu_dai_name[10]; /*  = "ssp[0|2]-port" */
+
+static bool is_valleyview(void)
+{
+	static const struct x86_cpu_id cpu_ids[] = {
+		{ X86_VENDOR_INTEL, 6, 55 }, /* Valleyview, Bay Trail */
+		{}
+	};
+
+	if (!x86_match_cpu(cpu_ids))
+		return false;
+	return true;
+}
+
+struct acpi_chan_package {   /* ACPICA seems to require 64 bit integers */
+	u64 aif_value;       /* 1: AIF1, 2: AIF2 */
+	u64 mclock_value;    /* usually 25MHz (0x17d7940), ignored */
+};
 
 static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 {
 	struct byt_rt5651_private *priv;
 	struct snd_soc_acpi_mach *mach;
 	const char *i2c_name = NULL;
+	bool is_bytcr = false;
 	int ret_val = 0;
 	int dai_index = 0;
 	int i;
@@ -640,6 +736,73 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 		"%s%s", "i2c-", i2c_name);
 	byt_rt5651_dais[dai_index].codec_name = byt_rt5651_codec_name;
 
+	/*
+	 * swap SSP0 if bytcr is detected
+	 * (will be overridden if DMI quirk is detected)
+	 */
+	if (is_valleyview()) {
+		struct sst_platform_info *p_info = mach->pdata;
+		const struct sst_res_info *res_info = p_info->res_info;
+
+		if (res_info->acpi_ipc_irq_index == 0)
+			is_bytcr = true;
+	}
+
+	if (is_bytcr) {
+		/*
+		 * Baytrail CR platforms may have CHAN package in BIOS, try
+		 * to find relevant routing quirk based as done on Windows
+		 * platforms. We have to read the information directly from the
+		 * BIOS, at this stage the card is not created and the links
+		 * with the codec driver/pdata are non-existent
+		 */
+
+		struct acpi_chan_package chan_package;
+
+		/* format specified: 2 64-bit integers */
+		struct acpi_buffer format = {sizeof("NN"), "NN"};
+		struct acpi_buffer state = {0, NULL};
+		struct snd_soc_acpi_package_context pkg_ctx;
+		bool pkg_found = false;
+
+		state.length = sizeof(chan_package);
+		state.pointer = &chan_package;
+
+		pkg_ctx.name = "CHAN";
+		pkg_ctx.length = 2;
+		pkg_ctx.format = &format;
+		pkg_ctx.state = &state;
+		pkg_ctx.data_valid = false;
+
+		pkg_found = snd_soc_acpi_find_package_from_hid(mach->id,
+							       &pkg_ctx);
+		if (pkg_found) {
+			if (chan_package.aif_value == 1) {
+				dev_info(&pdev->dev, "BIOS Routing: AIF1 connected\n");
+				byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF1;
+			} else  if (chan_package.aif_value == 2) {
+				dev_info(&pdev->dev, "BIOS Routing: AIF2 connected\n");
+				byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF2;
+			} else {
+				dev_info(&pdev->dev, "BIOS Routing isn't valid, ignored\n");
+				pkg_found = false;
+			}
+		}
+
+		if (!pkg_found) {
+			/* no BIOS indications, assume SSP0-AIF2 connection */
+			byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF2;
+		}
+
+		/* change defaults for Baytrail-CR capture */
+		byt_rt5651_quirk |= BYT_RT5651_JD1_1 |
+				    BYT_RT5651_OVCD_TH_2000UA |
+				    BYT_RT5651_OVCD_SF_0P75 |
+				    BYT_RT5651_IN2_HS_IN3_MAP;
+	} else {
+		byt_rt5651_quirk |= BYT_RT5651_DMIC_MAP;
+	}
+
 	/* check quirks before creating card */
 	dmi_check_system(byt_rt5651_quirk_table);
 
@@ -650,6 +813,28 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 
 	log_quirks(&pdev->dev);
 
+	if ((byt_rt5651_quirk & BYT_RT5651_SSP2_AIF2) ||
+	    (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) {
+		/* fixup codec aif name */
+		snprintf(byt_rt5651_codec_aif_name,
+			sizeof(byt_rt5651_codec_aif_name),
+			"%s", "rt5651-aif2");
+
+		byt_rt5651_dais[dai_index].codec_dai_name =
+			byt_rt5651_codec_aif_name;
+	}
+
+	if ((byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1) ||
+	    (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) {
+		/* fixup cpu dai name name */
+		snprintf(byt_rt5651_cpu_dai_name,
+			sizeof(byt_rt5651_cpu_dai_name),
+			"%s", "ssp0-port");
+
+		byt_rt5651_dais[dai_index].cpu_dai_name =
+			byt_rt5651_cpu_dai_name;
+	}
+
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) {
 		priv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
 		if (IS_ERR(priv->mclk)) {
-- 
2.14.3

Applied "ASoC: Intel: bytcr_rt5651: Add support for Bay Trail CR / SSP0 using boards" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Add support for Bay Trail CR / SSP0 using boards

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 8a880a2014e3ab50e66252335bd71d61b0487a30 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:36:07 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Add support for Bay Trail CR /
 SSP0 using boards

Despite its name being prefixed with bytcr, before this commit the
bytcr_rt5651 machine driver could not work with Bay Trail CR boards,
as those only have SSP0 and it only supported SSP0-AIF1 setups.

This commit adds support for this, autodetecting AIF1 vs AIF2 based on
BIOS tables.

While at it also add support for SSP2-AIF2 setups, as that requires only
minimal extra code on top of the code adding SSP0-AIF1 / SSP0-AIF2 support.

Note this code is all copy-pasted from bytcr_rt5640.c. I've looked into
merging the 2 machine drivers into 1 to avoid copy-pasting, but there are
enough subtile differences to make this hard *and* with all the quirks the
machine driver already is full with if (variant-foo) then ... else ...
constructs adding more of these is going to make the code unreadable.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 217 +++++++++++++++++++++++++++++++---
 1 file changed, 201 insertions(+), 16 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index b798ebb18bf1..103c4b0e6505 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -27,6 +27,7 @@
 #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 <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -72,6 +73,9 @@ enum {
 #define BYT_RT5651_DMIC_EN		BIT(16)
 #define BYT_RT5651_MCLK_EN		BIT(17)
 #define BYT_RT5651_MCLK_25MHZ		BIT(18)
+#define BYT_RT5651_SSP2_AIF2		BIT(19) /* default is using AIF1  */
+#define BYT_RT5651_SSP0_AIF1		BIT(20)
+#define BYT_RT5651_SSP0_AIF2		BIT(21)
 
 /* jack-detect-source + dmic-en + ovcd-th + -sf + terminating empty entry */
 #define MAX_NO_PROPS 5
@@ -81,8 +85,7 @@ struct byt_rt5651_private {
 	struct snd_soc_jack jack;
 };
 
-static unsigned long byt_rt5651_quirk = BYT_RT5651_DMIC_MAP |
-					BYT_RT5651_MCLK_EN;
+static unsigned long byt_rt5651_quirk = BYT_RT5651_MCLK_EN;
 
 static void log_quirks(struct device *dev)
 {
@@ -110,9 +113,16 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk MCLK_EN enabled");
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_25MHZ)
 		dev_info(dev, "quirk MCLK_25MHZ enabled");
+	if (byt_rt5651_quirk & BYT_RT5651_SSP2_AIF2)
+		dev_info(dev, "quirk SSP2_AIF2 enabled\n");
+	if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1)
+		dev_info(dev, "quirk SSP0_AIF1 enabled\n");
+	if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)
+		dev_info(dev, "quirk SSP0_AIF2 enabled\n");
 }
 
 #define BYT_CODEC_DAI1	"rt5651-aif1"
+#define BYT_CODEC_DAI2	"rt5651-aif2"
 
 static int byt_rt5651_prepare_and_enable_pll1(struct snd_soc_dai *codec_dai,
 					      int rate, int bclk_ratio)
@@ -156,6 +166,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 	int ret;
 
 	codec_dai = snd_soc_card_get_codec_dai(card, BYT_CODEC_DAI1);
+	if (!codec_dai)
+		codec_dai = snd_soc_card_get_codec_dai(card, BYT_CODEC_DAI2);
 	if (!codec_dai) {
 		dev_err(card->dev,
 			"Codec dai not found; Unable to set platform clock\n");
@@ -213,13 +225,6 @@ static const struct snd_soc_dapm_route byt_rt5651_audio_map[] = {
 	{"Speaker", NULL, "Platform Clock"},
 	{"Line In", NULL, "Platform Clock"},
 
-	{"AIF1 Playback", NULL, "ssp2 Tx"},
-	{"ssp2 Tx", NULL, "codec_out0"},
-	{"ssp2 Tx", NULL, "codec_out1"},
-	{"codec_in0", NULL, "ssp2 Rx"},
-	{"codec_in1", NULL, "ssp2 Rx"},
-	{"ssp2 Rx", NULL, "AIF1 Capture"},
-
 	{"Headset Mic", NULL, "micbias1"}, /* lowercase for rt5651 */
 	{"Headphone", NULL, "HPOL"},
 	{"Headphone", NULL, "HPOR"},
@@ -267,6 +272,42 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_in2_hs_in3_map[] = {
 	{"IN3P", NULL, "Headset Mic"},
 };
 
+static const struct snd_soc_dapm_route byt_rt5651_ssp0_aif1_map[] = {
+	{"ssp0 Tx", NULL, "modem_out"},
+	{"modem_in", NULL, "ssp0 Rx"},
+
+	{"AIF1 Playback", NULL, "ssp0 Tx"},
+	{"ssp0 Rx", NULL, "AIF1 Capture"},
+};
+
+static const struct snd_soc_dapm_route byt_rt5651_ssp0_aif2_map[] = {
+	{"ssp0 Tx", NULL, "modem_out"},
+	{"modem_in", NULL, "ssp0 Rx"},
+
+	{"AIF2 Playback", NULL, "ssp0 Tx"},
+	{"ssp0 Rx", NULL, "AIF2 Capture"},
+};
+
+static const struct snd_soc_dapm_route byt_rt5651_ssp2_aif1_map[] = {
+	{"ssp2 Tx", NULL, "codec_out0"},
+	{"ssp2 Tx", NULL, "codec_out1"},
+	{"codec_in0", NULL, "ssp2 Rx"},
+	{"codec_in1", NULL, "ssp2 Rx"},
+
+	{"AIF1 Playback", NULL, "ssp2 Tx"},
+	{"ssp2 Rx", NULL, "AIF1 Capture"},
+};
+
+static const struct snd_soc_dapm_route byt_rt5651_ssp2_aif2_map[] = {
+	{"ssp2 Tx", NULL, "codec_out0"},
+	{"ssp2 Tx", NULL, "codec_out1"},
+	{"codec_in0", NULL, "ssp2 Rx"},
+	{"codec_in1", NULL, "ssp2 Rx"},
+
+	{"AIF2 Playback", NULL, "ssp2 Tx"},
+	{"ssp2 Rx", NULL, "AIF2 Capture"},
+};
+
 static const struct snd_kcontrol_new byt_rt5651_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Headphone"),
 	SOC_DAPM_PIN_SWITCH("Headset Mic"),
@@ -291,9 +332,16 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	snd_pcm_format_t format = params_format(params);
 	int rate = params_rate(params);
+	int bclk_ratio;
 
-	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
+	if (format == SNDRV_PCM_FORMAT_S16_LE)
+		bclk_ratio = 32;
+	else
+		bclk_ratio = 50;
+
+	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, bclk_ratio);
 }
 
 static int byt_rt5651_quirk_cb(const struct dmi_system_id *id)
@@ -420,6 +468,26 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 	if (ret)
 		return ret;
 
+	if (byt_rt5651_quirk & BYT_RT5651_SSP2_AIF2) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp2_aif2_map,
+					ARRAY_SIZE(byt_rt5651_ssp2_aif2_map));
+	} else if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp0_aif1_map,
+					ARRAY_SIZE(byt_rt5651_ssp0_aif1_map));
+	} else if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp0_aif2_map,
+					ARRAY_SIZE(byt_rt5651_ssp0_aif2_map));
+	} else {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp2_aif1_map,
+					ARRAY_SIZE(byt_rt5651_ssp2_aif1_map));
+	}
+	if (ret)
+		return ret;
+
 	ret = snd_soc_add_card_controls(card, byt_rt5651_controls,
 					ARRAY_SIZE(byt_rt5651_controls));
 	if (ret) {
@@ -485,18 +553,26 @@ static int byt_rt5651_codec_fixup(struct snd_soc_pcm_runtime *rtd,
 			SNDRV_PCM_HW_PARAM_RATE);
 	struct snd_interval *channels = hw_param_interval(params,
 						SNDRV_PCM_HW_PARAM_CHANNELS);
-	int ret;
+	int ret, bits;
 
-	/* The DSP will covert the FE rate to 48k, stereo, 24bits */
+	/* The DSP will covert the FE rate to 48k, stereo */
 	rate->min = rate->max = 48000;
 	channels->min = channels->max = 2;
 
-	/* set SSP2 to 24-bit */
-	params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
+	if ((byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1) ||
+	    (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) {
+		/* set SSP0 to 16-bit */
+		params_set_format(params, SNDRV_PCM_FORMAT_S16_LE);
+		bits = 16;
+	} else {
+		/* set SSP2 to 24-bit */
+		params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
+		bits = 24;
+	}
 
 	/*
 	 * Default mode for SSP configuration is TDM 4 slot, override config
-	 * with explicit setting to I2S 2ch 24-bit. The word length is set with
+	 * with explicit setting to I2S 2ch. The word length is set with
 	 * dai_set_tdm_slot() since there is no other API exposed
 	 */
 	ret = snd_soc_dai_set_fmt(rtd->cpu_dai,
@@ -510,7 +586,7 @@ static int byt_rt5651_codec_fixup(struct snd_soc_pcm_runtime *rtd,
 		return ret;
 	}
 
-	ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, 24);
+	ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, bits);
 	if (ret < 0) {
 		dev_err(rtd->dev, "can't set I2S config, err %d\n", ret);
 		return ret;
@@ -605,12 +681,32 @@ static struct snd_soc_card byt_rt5651_card = {
 };
 
 static char byt_rt5651_codec_name[SND_ACPI_I2C_ID_LEN];
+static char byt_rt5651_codec_aif_name[12]; /*  = "rt5651-aif[1|2]" */
+static char byt_rt5651_cpu_dai_name[10]; /*  = "ssp[0|2]-port" */
+
+static bool is_valleyview(void)
+{
+	static const struct x86_cpu_id cpu_ids[] = {
+		{ X86_VENDOR_INTEL, 6, 55 }, /* Valleyview, Bay Trail */
+		{}
+	};
+
+	if (!x86_match_cpu(cpu_ids))
+		return false;
+	return true;
+}
+
+struct acpi_chan_package {   /* ACPICA seems to require 64 bit integers */
+	u64 aif_value;       /* 1: AIF1, 2: AIF2 */
+	u64 mclock_value;    /* usually 25MHz (0x17d7940), ignored */
+};
 
 static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 {
 	struct byt_rt5651_private *priv;
 	struct snd_soc_acpi_mach *mach;
 	const char *i2c_name = NULL;
+	bool is_bytcr = false;
 	int ret_val = 0;
 	int dai_index = 0;
 	int i;
@@ -643,6 +739,73 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 		"%s%s", "i2c-", i2c_name);
 	byt_rt5651_dais[dai_index].codec_name = byt_rt5651_codec_name;
 
+	/*
+	 * swap SSP0 if bytcr is detected
+	 * (will be overridden if DMI quirk is detected)
+	 */
+	if (is_valleyview()) {
+		struct sst_platform_info *p_info = mach->pdata;
+		const struct sst_res_info *res_info = p_info->res_info;
+
+		if (res_info->acpi_ipc_irq_index == 0)
+			is_bytcr = true;
+	}
+
+	if (is_bytcr) {
+		/*
+		 * Baytrail CR platforms may have CHAN package in BIOS, try
+		 * to find relevant routing quirk based as done on Windows
+		 * platforms. We have to read the information directly from the
+		 * BIOS, at this stage the card is not created and the links
+		 * with the codec driver/pdata are non-existent
+		 */
+
+		struct acpi_chan_package chan_package;
+
+		/* format specified: 2 64-bit integers */
+		struct acpi_buffer format = {sizeof("NN"), "NN"};
+		struct acpi_buffer state = {0, NULL};
+		struct snd_soc_acpi_package_context pkg_ctx;
+		bool pkg_found = false;
+
+		state.length = sizeof(chan_package);
+		state.pointer = &chan_package;
+
+		pkg_ctx.name = "CHAN";
+		pkg_ctx.length = 2;
+		pkg_ctx.format = &format;
+		pkg_ctx.state = &state;
+		pkg_ctx.data_valid = false;
+
+		pkg_found = snd_soc_acpi_find_package_from_hid(mach->id,
+							       &pkg_ctx);
+		if (pkg_found) {
+			if (chan_package.aif_value == 1) {
+				dev_info(&pdev->dev, "BIOS Routing: AIF1 connected\n");
+				byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF1;
+			} else  if (chan_package.aif_value == 2) {
+				dev_info(&pdev->dev, "BIOS Routing: AIF2 connected\n");
+				byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF2;
+			} else {
+				dev_info(&pdev->dev, "BIOS Routing isn't valid, ignored\n");
+				pkg_found = false;
+			}
+		}
+
+		if (!pkg_found) {
+			/* no BIOS indications, assume SSP0-AIF2 connection */
+			byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF2;
+		}
+
+		/* change defaults for Baytrail-CR capture */
+		byt_rt5651_quirk |= BYT_RT5651_JD1_1 |
+				    BYT_RT5651_OVCD_TH_2000UA |
+				    BYT_RT5651_OVCD_SF_0P75 |
+				    BYT_RT5651_IN2_HS_IN3_MAP;
+	} else {
+		byt_rt5651_quirk |= BYT_RT5651_DMIC_MAP;
+	}
+
 	/* check quirks before creating card */
 	dmi_check_system(byt_rt5651_quirk_table);
 
@@ -653,6 +816,28 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 
 	log_quirks(&pdev->dev);
 
+	if ((byt_rt5651_quirk & BYT_RT5651_SSP2_AIF2) ||
+	    (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) {
+		/* fixup codec aif name */
+		snprintf(byt_rt5651_codec_aif_name,
+			sizeof(byt_rt5651_codec_aif_name),
+			"%s", "rt5651-aif2");
+
+		byt_rt5651_dais[dai_index].codec_dai_name =
+			byt_rt5651_codec_aif_name;
+	}
+
+	if ((byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1) ||
+	    (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) {
+		/* fixup cpu dai name name */
+		snprintf(byt_rt5651_cpu_dai_name,
+			sizeof(byt_rt5651_cpu_dai_name),
+			"%s", "ssp0-port");
+
+		byt_rt5651_dais[dai_index].cpu_dai_name =
+			byt_rt5651_cpu_dai_name;
+	}
+
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) {
 		priv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
 		if (IS_ERR(priv->mclk)) {
-- 
2.16.2

Re: [PATCH v3 19/22] ASoC: Intel: bytcr_rt5651: Add support for Bay Trail CR / SSP0 using boards

[Pierre-Louis Bossart]

Sorry about the delay, I've only found the time to look at this machine 
driver changes this afternoon while I was doing the SOF integration

> @@ -291,9 +332,16 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
>   {
>   	struct snd_soc_pcm_runtime *rtd = substream->private_data;
>   	struct snd_soc_dai *codec_dai = rtd->codec_dai;
> +	snd_pcm_format_t format = params_format(params);
>   	int rate = params_rate(params);
> +	int bclk_ratio;
>   
> -	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
> +	if (format == SNDRV_PCM_FORMAT_S16_LE)
> +		bclk_ratio = 32;

I don't think we can generate a 32x ratio with a 19.2 MHz clock, even 
less so with a 25MHz reference.
And in addition in the bytcr_rt5640 case we could never get any audio on 
bytcr which uses the SSP0 link without the mclk enabled, so this 
SSP0+blck mode was never tested (maybe precisely because this ratio was 
wrong...)

In other words there is a test gap here for the case using SSP0+bclk as 
a reference with the rt5651 as well. Does anyone have access to a BYT-CR 
platform w/ rt5651 to test this?

The other comment I have is that this run-time definition of the 
bclk_ratio is inconsistent with the hard-coded value I see in 
platform_clock_control()
ret = byt_rt5651_prepare_and_enable_pll1(codec_dai, 48000, 50);

Looks good otherwise.
Cheers
-Pierre

Re: [PATCH v3 19/22] ASoC: Intel: bytcr_rt5651: Add support for Bay Trail CR / SSP0 using boards

[Hans de Goede]

Hi,

On 10-03-18 00:30, Pierre-Louis Bossart wrote:
> 
> Sorry about the delay, I've only found the time to look at this machine driver changes this afternoon while I was doing the SOF integration
> 
>> @@ -291,9 +332,16 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
>>   {
>>       struct snd_soc_pcm_runtime *rtd = substream->private_data;
>>       struct snd_soc_dai *codec_dai = rtd->codec_dai;
>> +    snd_pcm_format_t format = params_format(params);
>>       int rate = params_rate(params);
>> +    int bclk_ratio;
>> -    return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
>> +    if (format == SNDRV_PCM_FORMAT_S16_LE)
>> +        bclk_ratio = 32;
> 
> I don't think we can generate a 32x ratio with a 19.2 MHz clock, even less so with a 25MHz reference.
> And in addition in the bytcr_rt5640 case we could never get any audio on bytcr which uses the SSP0 link without the mclk enabled, so this SSP0+blck mode was never tested (maybe precisely because this ratio was wrong...)
> 
> In other words there is a test gap here for the case using SSP0+bclk as a reference with the rt5651 as well. Does anyone have access to a BYT-CR platform w/ rt5651 to test this?

Yes I've access to a BYT-CR platform w/ rt5651, I assume that you want me to test there with the
BYT_RT5651_MCLK_EN quirk unset?

What should I try to fix things if they don't work?  Or maybe just put a big fat
warning printf in the driver if SSP0 is used without bclk and remove the handling
for the bclk + SSP0 case as its broken anyways ?

> The other comment I have is that this run-time definition of the bclk_ratio is inconsistent with the hard-coded value I see in platform_clock_control()
> ret = byt_rt5651_prepare_and_enable_pll1(codec_dai, 48000, 50);

Yes the assumption here is that we simply need "a" clock when the "Platform Clock"
gets enabled and that the clock then will later get set to the correct speed.

AFAIK you did not want duplication of the SSP0 handling outside the fixup
function and platform_clock_control() does not have access to the format,
since that is likely not even setup by then.

Regards,

Hans

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

Re: [PATCH v3 19/22] ASoC: Intel: bytcr_rt5651: Add support for Bay Trail CR / SSP0 using boards

[Pierre-Louis Bossart]

On 3/11/18 1:19 PM, Hans de Goede wrote:
> Hi,
> 
> On 10-03-18 00:30, Pierre-Louis Bossart wrote:
>>
>> Sorry about the delay, I've only found the time to look at this 
>> machine driver changes this afternoon while I was doing the SOF 
>> integration
>>
>>> @@ -291,9 +332,16 @@ static int byt_rt5651_aif1_hw_params(struct 
>>> snd_pcm_substream *substream,
>>>   {
>>>       struct snd_soc_pcm_runtime *rtd = substream->private_data;
>>>       struct snd_soc_dai *codec_dai = rtd->codec_dai;
>>> +    snd_pcm_format_t format = params_format(params);
>>>       int rate = params_rate(params);
>>> +    int bclk_ratio;
>>> -    return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
>>> +    if (format == SNDRV_PCM_FORMAT_S16_LE)
>>> +        bclk_ratio = 32;
>>
>> I don't think we can generate a 32x ratio with a 19.2 MHz clock, even 
>> less so with a 25MHz reference.
>> And in addition in the bytcr_rt5640 case we could never get any audio 
>> on bytcr which uses the SSP0 link without the mclk enabled, so this 
>> SSP0+blck mode was never tested (maybe precisely because this ratio 
>> was wrong...)
>>
>> In other words there is a test gap here for the case using SSP0+bclk 
>> as a reference with the rt5651 as well. Does anyone have access to a 
>> BYT-CR platform w/ rt5651 to test this?
> 
> Yes I've access to a BYT-CR platform w/ rt5651, I assume that you want 
> me to test there with the
> BYT_RT5651_MCLK_EN quirk unset?

yes.

> 
> What should I try to fix things if they don't work?  Or maybe just put a 
> big fat
> warning printf in the driver if SSP0 is used without bclk and remove the 
> handling
> for the bclk + SSP0 case as its broken anyways ?

Dunno. We may be able to test with SOF as well and check what formats work.

> 
>> The other comment I have is that this run-time definition of the 
>> bclk_ratio is inconsistent with the hard-coded value I see in 
>> platform_clock_control()
>> ret = byt_rt5651_prepare_and_enable_pll1(codec_dai, 48000, 50);
> 
> Yes the assumption here is that we simply need "a" clock when the 
> "Platform Clock"
> gets enabled and that the clock then will later get set to the correct 
> speed.
> 
> AFAIK you did not want duplication of the SSP0 handling outside the fixup
> function and platform_clock_control() does not have access to the format,
> since that is likely not even setup by then.

well things are broken in the 16-bit mode then?

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

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

[PATCH v3 20/22] ASoC: Intel: bytcr_rt5651: Add quirk for the VIOS LTH17 laptop

[Hans de Goede]

Add a quirk setting up jack-detect and input routing for the
VIOS LTH17 laptop.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 1f48b2920f48..4c150ce2c8f0 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -393,6 +393,19 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 					BYT_RT5651_OVCD_SF_0P75 |
 					BYT_RT5651_IN2_HS_IN3_MAP),
 	},
+	{
+		/* VIOS LTH17 */
+		.callback = byt_rt5651_quirk_cb,
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "VIOS"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "LTH17"),
+		},
+		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_1P0 |
+					BYT_RT5651_IN1_IN2_MAP),
+	},
 	{}
 };
 
-- 
2.14.3

[PATCH v3 21/22] ASoC: Intel: bytcr_rt5651: Change defaults to enable jack-detect, analog mics

[Hans de Goede]

Change the default quirk settings to enable jack-detect, analog mics.

The old default input mapping of DMIC for non Bay Trail CR devices seems
like a poor default as I'm not aware of any Intel SST + rt5651 using
devices with a DMIC.

All Cherry Trail devices using the bytcr_rt5651 machine driver seem to be
modelled after BYT-CR devices, And the only non CR Bay Trail devices with
a rt5651 codec I'm aware of are the Minnow boards for which we already have
board specific quirks. So it seems better to me to use the BYT-CR defaults
everywhere.

This e.g. makes the Chuwi Hi8 Pro (CWI513) work ootb without needing a
quirk.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 4c150ce2c8f0..b35ec60b21bf 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -85,7 +85,12 @@ struct byt_rt5651_private {
 	struct snd_soc_jack jack;
 };
 
-static unsigned long byt_rt5651_quirk = BYT_RT5651_MCLK_EN;
+/* Default: jack-detect on JD1_1, internal mic on in2, headsetmic on in3 */
+static unsigned long byt_rt5651_quirk = BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_0P75 |
+					BYT_RT5651_IN2_HS_IN3_MAP;
 
 static void log_quirks(struct device *dev)
 {
@@ -806,14 +811,6 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 			/* no BIOS indications, assume SSP0-AIF2 connection */
 			byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF2;
 		}
-
-		/* change defaults for Baytrail-CR capture */
-		byt_rt5651_quirk |= BYT_RT5651_JD1_1 |
-				    BYT_RT5651_OVCD_TH_2000UA |
-				    BYT_RT5651_OVCD_SF_0P75 |
-				    BYT_RT5651_IN2_HS_IN3_MAP;
-	} else {
-		byt_rt5651_quirk |= BYT_RT5651_DMIC_MAP;
 	}
 
 	/* check quirks before creating card */
-- 
2.14.3

[PATCH v3 22/22] ASoC: Intel: bytcr_rt5651: Select RCCLK on init()

[Hans de Goede]

When the BYT_RT5651_MCLK_EN quirk is set, we disable the MCLK from
byt_rt5651_init(), we need to select the RCCLK as sysclk before doing this
to make sure that jack-detect works directly after boot.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index b35ec60b21bf..65a81ce83f17 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -457,6 +457,11 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 
 	card->dapm.idle_bias_off = true;
 
+	/* Start with RC clk for jack-detect (we disable MCLK below) */
+	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN)
+		snd_soc_component_update_bits(codec, RT5651_GLB_CLK,
+			RT5651_SCLK_SRC_MASK, RT5651_SCLK_SRC_RCCLK);
+
 	switch (BYT_RT5651_MAP(byt_rt5651_quirk)) {
 	case BYT_RT5651_IN1_MAP:
 		custom_map = byt_rt5651_intmic_in1_map;
-- 
2.14.3