[PATCH 0/5] ASoC: rt286/rt298: Fixes for DMIC2 config and combo jack

[PATCH 0/5] ASoC: rt286/rt298: Fixes for DMIC2 config and combo jack

[David Ward]

The last two patches in this series fix a longstanding issue that prevented
the ALC3263 codec from using a headset mic. This codec can be found on Dell
systems including the Latitude 13 7350, Venue 11 Pro 7140, and XPS 13 9343.
In fact, there is an ACPI quirk for the XPS 13 9343, which forces it to use
legacy HD Audio just to avoid this issue:

https://lore.kernel.org/alsa-devel/CAPeXnHv07HkvcHrYFmZMr8OTp7U7F=k_k=LPYnUtp89iPn2d2Q@mail.gmail.com/

This may allow that ACPI quirk to be removed. Either way, the other systems
mentioned above do not support this quirk and already use the ASoC driver,
so this fix is necessary for headset mic support on those systems.

Note: there is likely other handling for this codec that only exists in the
HDA driver, but which also belongs in the ASoC driver. Commit 394c97f824fa
("ALSA: hda/realtek - Change EAPD to verb control") describes an issue that
does not seem to be resolved in the ASoC driver, to give an example.


Other patches in this series are not specific to the ALC3263. These patches
set the correct combo jack configuration when headphones are inserted, and
fix a misaligned value set in the DMIC2 Configuration Default register.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=114171
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=150601
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=205961
Signed-off-by: David Ward <david.ward@gatech.edu>

David Ward (5):
  ASoC: rt286: Fix upper byte in DMIC2 configuration
  ASoC: rt286: Configure combo jack for headphones
  ASoC: rt298: Configure combo jack for headphones
  ASoC: rt286: Make RT286_SET_GPIO_* readable and writable
  ASoC: rt286: Generalize support for ALC3263 codec

 sound/soc/codecs/rt286.c | 34 +++++++++++++++++++++-------------
 sound/soc/codecs/rt298.c |  9 +++++++--
 2 files changed, 28 insertions(+), 15 deletions(-)

-- 
2.31.1

[PATCH 1/5] ASoC: rt286: Fix upper byte in DMIC2 configuration

[David Ward]

This HDA verb sets the upper byte of the Configuration Default register, so
only an 8-bit value should be used. For the rt298, the same fix was applied
in commit f8f2dc4a7127 ("ASoC: rt298: fix wrong setting of gpio2_en").

Signed-off-by: David Ward <david.ward@gatech.edu>
---
 sound/soc/codecs/rt286.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt286.c b/sound/soc/codecs/rt286.c
index 8abe232ca4a4..f9b29782b62a 100644
--- a/sound/soc/codecs/rt286.c
+++ b/sound/soc/codecs/rt286.c
@@ -1204,7 +1204,7 @@ static int rt286_i2c_probe(struct i2c_client *i2c,
 	mdelay(10);
 
 	if (!rt286->pdata.gpio2_en)
-		regmap_write(rt286->regmap, RT286_SET_DMIC2_DEFAULT, 0x4000);
+		regmap_write(rt286->regmap, RT286_SET_DMIC2_DEFAULT, 0x40);
 	else
 		regmap_write(rt286->regmap, RT286_SET_DMIC2_DEFAULT, 0);
 
-- 
2.31.1

[PATCH 2/5] ASoC: rt286: Configure combo jack for headphones

[David Ward]

During jack detection, the combo jack is configured for a CTIA headset, and
then for an OMTP headset, while sensing the mic connection. If a mic is not
found in either case, the combo jack should be re-configured for headphones
only. This is consistent with the HDA driver behavior.

Signed-off-by: David Ward <david.ward@gatech.edu>
---
 sound/soc/codecs/rt286.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/rt286.c b/sound/soc/codecs/rt286.c
index f9b29782b62a..67a5affdb705 100644
--- a/sound/soc/codecs/rt286.c
+++ b/sound/soc/codecs/rt286.c
@@ -252,11 +252,16 @@ static int rt286_jack_detect(struct rt286_priv *rt286, bool *hp, bool *mic)
 				msleep(300);
 				regmap_read(rt286->regmap,
 					RT286_CBJ_CTRL2, &val);
-				if (0x0070 == (val & 0x0070))
+				if (0x0070 == (val & 0x0070)) {
 					*mic = true;
-				else
+				} else {
 					*mic = false;
+					regmap_update_bits(rt286->regmap,
+						RT286_CBJ_CTRL1,
+						0xfcc0, 0xc400);
+				}
 			}
+
 			regmap_update_bits(rt286->regmap,
 				RT286_DC_GAIN, 0x200, 0x0);
 
-- 
2.31.1

[PATCH 3/5] ASoC: rt298: Configure combo jack for headphones

[David Ward]

During jack detection, the combo jack is configured for a CTIA headset, and
then for an OMTP headset, while sensing the mic connection. If a mic is not
found in either case, the combo jack should be re-configured for headphones
only. This is consistent with the HDA driver behavior.

Signed-off-by: David Ward <david.ward@gatech.edu>
---
 sound/soc/codecs/rt298.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/rt298.c b/sound/soc/codecs/rt298.c
index 32cc9b6287d2..c592c40a7ab3 100644
--- a/sound/soc/codecs/rt298.c
+++ b/sound/soc/codecs/rt298.c
@@ -267,11 +267,16 @@ static int rt298_jack_detect(struct rt298_priv *rt298, bool *hp, bool *mic)
 				msleep(300);
 				regmap_read(rt298->regmap,
 					RT298_CBJ_CTRL2, &val);
-				if (0x0070 == (val & 0x0070))
+				if (0x0070 == (val & 0x0070)) {
 					*mic = true;
-				else
+				} else {
 					*mic = false;
+					regmap_update_bits(rt298->regmap,
+						RT298_CBJ_CTRL1,
+						0xfcc0, 0xc400);
+				}
 			}
+
 			regmap_update_bits(rt298->regmap,
 				RT298_DC_GAIN, 0x200, 0x0);
 
-- 
2.31.1

[PATCH 4/5] ASoC: rt286: Make RT286_SET_GPIO_* readable and writable

[David Ward]

The GPIO configuration cannot be applied if the registers are inaccessible.
This prevented the headset mic from working on the Dell XPS 13 9343.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=114171
Signed-off-by: David Ward <david.ward@gatech.edu>
---
 sound/soc/codecs/rt286.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/codecs/rt286.c b/sound/soc/codecs/rt286.c
index 67a5affdb705..45e4a48ef5bf 100644
--- a/sound/soc/codecs/rt286.c
+++ b/sound/soc/codecs/rt286.c
@@ -171,6 +171,9 @@ static bool rt286_readable_register(struct device *dev, unsigned int reg)
 	case RT286_PROC_COEF:
 	case RT286_SET_AMP_GAIN_ADC_IN1:
 	case RT286_SET_AMP_GAIN_ADC_IN2:
+	case RT286_SET_GPIO_MASK:
+	case RT286_SET_GPIO_DIRECTION:
+	case RT286_SET_GPIO_DATA:
 	case RT286_SET_POWER(RT286_DAC_OUT1):
 	case RT286_SET_POWER(RT286_DAC_OUT2):
 	case RT286_SET_POWER(RT286_ADC_IN1):
-- 
2.31.1

[PATCH 5/5] ASoC: rt286: Generalize support for ALC3263 codec

[David Ward]

The ALC3263 codec on the XPS 13 9343 is also found on the Latitude 13 7350
and Venue 11 Pro 7140. They require the same handling for the combo jack to
work with a headset: GPIO pin 6 must be set.

The HDA driver always sets this pin on the ALC3263, which it distinguishes
by the codec vendor/device ID 0x10ec0288 and PCI subsystem vendor ID 0x1028
(Dell). The ASoC driver does not use PCI, so adapt this check to use DMI to
determine if Dell is the system vendor.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=150601
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=205961
Signed-off-by: David Ward <david.ward@gatech.edu>
---
 sound/soc/codecs/rt286.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/sound/soc/codecs/rt286.c b/sound/soc/codecs/rt286.c
index 45e4a48ef5bf..802f4851c3df 100644
--- a/sound/soc/codecs/rt286.c
+++ b/sound/soc/codecs/rt286.c
@@ -1125,12 +1125,11 @@ static const struct dmi_system_id force_combo_jack_table[] = {
 	{ }
 };
 
-static const struct dmi_system_id dmi_dell_dino[] = {
+static const struct dmi_system_id dmi_dell[] = {
 	{
-		.ident = "Dell Dino",
+		.ident = "Dell",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "XPS 13 9343")
 		}
 	},
 	{ }
@@ -1141,7 +1140,7 @@ static int rt286_i2c_probe(struct i2c_client *i2c,
 {
 	struct rt286_platform_data *pdata = dev_get_platdata(&i2c->dev);
 	struct rt286_priv *rt286;
-	int i, ret, val;
+	int i, ret, vendor_id;
 
 	rt286 = devm_kzalloc(&i2c->dev,	sizeof(*rt286),
 				GFP_KERNEL);
@@ -1157,14 +1156,15 @@ static int rt286_i2c_probe(struct i2c_client *i2c,
 	}
 
 	ret = regmap_read(rt286->regmap,
-		RT286_GET_PARAM(AC_NODE_ROOT, AC_PAR_VENDOR_ID), &val);
+		RT286_GET_PARAM(AC_NODE_ROOT, AC_PAR_VENDOR_ID), &vendor_id);
 	if (ret != 0) {
 		dev_err(&i2c->dev, "I2C error %d\n", ret);
 		return ret;
 	}
-	if (val != RT286_VENDOR_ID && val != RT288_VENDOR_ID) {
+	if (vendor_id != RT286_VENDOR_ID && vendor_id != RT288_VENDOR_ID) {
 		dev_err(&i2c->dev,
-			"Device with ID register %#x is not rt286\n", val);
+			"Device with ID register %#x is not rt286\n",
+			vendor_id);
 		return -ENODEV;
 	}
 
@@ -1188,8 +1188,8 @@ static int rt286_i2c_probe(struct i2c_client *i2c,
 	if (pdata)
 		rt286->pdata = *pdata;
 
-	if (dmi_check_system(force_combo_jack_table) ||
-		dmi_check_system(dmi_dell_dino))
+	if ((vendor_id == RT288_VENDOR_ID && dmi_check_system(dmi_dell)) ||
+		dmi_check_system(force_combo_jack_table))
 		rt286->pdata.cbj_en = true;
 
 	regmap_write(rt286->regmap, RT286_SET_AUDIO_POWER, AC_PWRST_D3);
@@ -1228,7 +1228,7 @@ static int rt286_i2c_probe(struct i2c_client *i2c,
 	regmap_update_bits(rt286->regmap, RT286_DEPOP_CTRL3, 0xf777, 0x4737);
 	regmap_update_bits(rt286->regmap, RT286_DEPOP_CTRL4, 0x00ff, 0x003f);
 
-	if (dmi_check_system(dmi_dell_dino)) {
+	if (vendor_id == RT288_VENDOR_ID && dmi_check_system(dmi_dell)) {
 		regmap_update_bits(rt286->regmap,
 			RT286_SET_GPIO_MASK, 0x40, 0x40);
 		regmap_update_bits(rt286->regmap,
-- 
2.31.1

Re: [PATCH 0/5] ASoC: rt286/rt298: Fixes for DMIC2 config and combo jack

[Pierre-Louis Bossart]

On 4/18/21 8:46 AM, David Ward wrote:
> The last two patches in this series fix a longstanding issue that prevented
> the ALC3263 codec from using a headset mic. This codec can be found on Dell
> systems including the Latitude 13 7350, Venue 11 Pro 7140, and XPS 13 9343.
> In fact, there is an ACPI quirk for the XPS 13 9343, which forces it to use
> legacy HD Audio just to avoid this issue:
> 
> https://lore.kernel.org/alsa-devel/CAPeXnHv07HkvcHrYFmZMr8OTp7U7F=k_k=LPYnUtp89iPn2d2Q@mail.gmail.com/
> 
> This may allow that ACPI quirk to be removed. Either way, the other systems
> mentioned above do not support this quirk and already use the ASoC driver,
> so this fix is necessary for headset mic support on those systems.

You should probably add a Fixes tag for the 5 patches so that the 
changes make their way into the -stable kernel that most distributions use?

The patches themselves look ok to me

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> 
> Note: there is likely other handling for this codec that only exists in the
> HDA driver, but which also belongs in the ASoC driver. Commit 394c97f824fa
> ("ALSA: hda/realtek - Change EAPD to verb control") describes an issue that
> does not seem to be resolved in the ASoC driver, to give an example.
> 
> 
> Other patches in this series are not specific to the ALC3263. These patches
> set the correct combo jack configuration when headphones are inserted, and
> fix a misaligned value set in the DMIC2 Configuration Default register.
> 
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=114171
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=150601
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=205961
> Signed-off-by: David Ward <david.ward@gatech.edu>
> 
> David Ward (5):
>    ASoC: rt286: Fix upper byte in DMIC2 configuration
>    ASoC: rt286: Configure combo jack for headphones
>    ASoC: rt298: Configure combo jack for headphones
>    ASoC: rt286: Make RT286_SET_GPIO_* readable and writable
>    ASoC: rt286: Generalize support for ALC3263 codec
> 
>   sound/soc/codecs/rt286.c | 34 +++++++++++++++++++++-------------
>   sound/soc/codecs/rt298.c |  9 +++++++--
>   2 files changed, 28 insertions(+), 15 deletions(-)
> 

Re: [PATCH 0/5] ASoC: rt286/rt298: Fixes for DMIC2 config and combo jack

[Mark Brown]

On Sun, 18 Apr 2021 09:46:53 -0400, David Ward wrote:
> The last two patches in this series fix a longstanding issue that prevented
> the ALC3263 codec from using a headset mic. This codec can be found on Dell
> systems including the Latitude 13 7350, Venue 11 Pro 7140, and XPS 13 9343.
> In fact, there is an ACPI quirk for the XPS 13 9343, which forces it to use
> legacy HD Audio just to avoid this issue:
> 
> https://lore.kernel.org/alsa-devel/CAPeXnHv07HkvcHrYFmZMr8OTp7U7F=k_k=LPYnUtp89iPn2d2Q@mail.gmail.com/
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/5] ASoC: rt286: Fix upper byte in DMIC2 configuration
      commit: dfa7b01dbdc9723ced606425b47005bb583a8778
[2/5] ASoC: rt286: Configure combo jack for headphones
      commit: b15c3ea7e0d2c53b33adc8ca0896de378d6df853
[3/5] ASoC: rt298: Configure combo jack for headphones
      commit: f3bd4dde9f048fa711e98626c8c3bc4d1eae3436
[4/5] ASoC: rt286: Make RT286_SET_GPIO_* readable and writable
      commit: cd8499d5c03ba260e3191e90236d0e5f6b147563
[5/5] ASoC: rt286: Generalize support for ALC3263 codec
      commit: aa2f9c12821e6a4ba1df4fb34a3dbc6a2a1ee7fe

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