[PATCH 0/8] Few audio fixes on Tegra platforms

[PATCH 0/8] Few audio fixes on Tegra platforms

[Sameer Pujar]

This series fixes some of the issues which were observed during an attempt to
enhance automated test coverage on Jetson AGX Orin. Below is a short summary
of the issues and fixes:

  * Sample rate coversion failures above 48kHz.
  * AMX and ADX test cases failures due to incorrect byte mask.
  * Atomic sleep in RT5640 codec which is present on Jetson AGX Orin.
  * AHUB clock fixes on Tegra234 and previous chips.
  * Minor cleanups in ASRC and AHUB driver.


Sameer Pujar (4):
  ASoC: rt5640: Fix sleep in atomic context
  ASoC: tegra: Use normal system sleep for ASRC
  ASoC: tegra: Remove stale comments in AHUB
  arm64: tegra: Update AHUB clock parent and rate

Sheetal (4):
  ASoC: tegra: Fix SFC conversion for few rates
  ASoC: tegra: Fix AMX byte map
  ASoC: tegra: Fix ADX byte map
  arm64: tegra: Update AHUB clock parent and rate on Tegra234

 arch/arm64/boot/dts/nvidia/tegra186.dtsi |  3 ++-
 arch/arm64/boot/dts/nvidia/tegra194.dtsi |  3 ++-
 arch/arm64/boot/dts/nvidia/tegra210.dtsi |  3 ++-
 arch/arm64/boot/dts/nvidia/tegra234.dtsi |  3 ++-
 sound/soc/codecs/rt5640.c                | 20 ++++++++++++--------
 sound/soc/tegra/tegra186_asrc.c          |  4 ++--
 sound/soc/tegra/tegra210_adx.c           | 24 ++++++++++++------------
 sound/soc/tegra/tegra210_ahub.c          | 10 ----------
 sound/soc/tegra/tegra210_amx.c           | 30 ++++++++++++------------------
 sound/soc/tegra/tegra210_sfc.c           | 31 ++++++++++++++++++++++++++++++-
 sound/soc/tegra/tegra210_sfc.h           |  4 ++--
 11 files changed, 78 insertions(+), 57 deletions(-)

-- 
2.7.4

[PATCH 1/8] ASoC: tegra: Fix SFC conversion for few rates

[Sameer Pujar]

From: Sheetal <sheetal@nvidia.com>

Sample rate conversions for rates greater than 48kHz are found to be
failing. It means x->y conversions fail when either x or y is greater
than 48kHz.

This happens because, tegra210_sfc_rate_to_idx() returns incorrect
index for rates greater than 48kHz. This actually depends on the
tegra210_sfc_rates[] array and it is not in sync with frequency
values of SFC TX/RX register. To be precise, 64kHz entry is missing
in above array defined in the driver. Due to this wrong index is
returned and this results in incorrect programming of coefficients.

To fix this, align the tegra210_sfc_rates[] array with SFC register
specification and thus add 64kHz entry to it. Also, the coefficient
table is updated to reflect that none of the conversions are supported
for 64kHz.

Fixes: b2f74ec53a6c ("ASoC: tegra: Add Tegra210 based SFC driver")
Cc: stable@vger.kernel.org
Signed-off-by: Sheetal <sheetal@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
Reviewed-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/soc/tegra/tegra210_sfc.c | 31 ++++++++++++++++++++++++++++++-
 sound/soc/tegra/tegra210_sfc.h |  4 ++--
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/sound/soc/tegra/tegra210_sfc.c b/sound/soc/tegra/tegra210_sfc.c
index e9df1ff..c2240bab 100644
--- a/sound/soc/tegra/tegra210_sfc.c
+++ b/sound/soc/tegra/tegra210_sfc.c
@@ -2,7 +2,7 @@
 //
 // tegra210_sfc.c - Tegra210 SFC driver
 //
-// Copyright (c) 2021 NVIDIA CORPORATION.  All rights reserved.
+// Copyright (c) 2021-2023 NVIDIA CORPORATION.  All rights reserved.
 
 #include <linux/clk.h>
 #include <linux/device.h>
@@ -42,6 +42,7 @@ static const int tegra210_sfc_rates[TEGRA210_SFC_NUM_RATES] = {
 	32000,
 	44100,
 	48000,
+	64000,
 	88200,
 	96000,
 	176400,
@@ -2857,6 +2858,7 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		coef_8to32,
 		coef_8to44,
 		coef_8to48,
+		UNSUPP_CONV,
 		coef_8to88,
 		coef_8to96,
 		UNSUPP_CONV,
@@ -2872,6 +2874,7 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		coef_11to32,
 		coef_11to44,
 		coef_11to48,
+		UNSUPP_CONV,
 		coef_11to88,
 		coef_11to96,
 		UNSUPP_CONV,
@@ -2887,6 +2890,7 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		coef_16to32,
 		coef_16to44,
 		coef_16to48,
+		UNSUPP_CONV,
 		coef_16to88,
 		coef_16to96,
 		coef_16to176,
@@ -2902,6 +2906,7 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		coef_22to32,
 		coef_22to44,
 		coef_22to48,
+		UNSUPP_CONV,
 		coef_22to88,
 		coef_22to96,
 		coef_22to176,
@@ -2917,6 +2922,7 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		coef_24to32,
 		coef_24to44,
 		coef_24to48,
+		UNSUPP_CONV,
 		coef_24to88,
 		coef_24to96,
 		coef_24to176,
@@ -2932,6 +2938,7 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		BYPASS_CONV,
 		coef_32to44,
 		coef_32to48,
+		UNSUPP_CONV,
 		coef_32to88,
 		coef_32to96,
 		coef_32to176,
@@ -2947,6 +2954,7 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		coef_44to32,
 		BYPASS_CONV,
 		coef_44to48,
+		UNSUPP_CONV,
 		coef_44to88,
 		coef_44to96,
 		coef_44to176,
@@ -2962,11 +2970,28 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		coef_48to32,
 		coef_48to44,
 		BYPASS_CONV,
+		UNSUPP_CONV,
 		coef_48to88,
 		coef_48to96,
 		coef_48to176,
 		coef_48to192,
 	},
+	/* Convertions from 64 kHz */
+	{
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+		UNSUPP_CONV,
+	},
 	/* Convertions from 88.2 kHz */
 	{
 		coef_88to8,
@@ -2977,6 +3002,7 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		coef_88to32,
 		coef_88to44,
 		coef_88to48,
+		UNSUPP_CONV,
 		BYPASS_CONV,
 		coef_88to96,
 		coef_88to176,
@@ -2991,6 +3017,7 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		coef_96to32,
 		coef_96to44,
 		coef_96to48,
+		UNSUPP_CONV,
 		coef_96to88,
 		BYPASS_CONV,
 		coef_96to176,
@@ -3006,6 +3033,7 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		coef_176to32,
 		coef_176to44,
 		coef_176to48,
+		UNSUPP_CONV,
 		coef_176to88,
 		coef_176to96,
 		BYPASS_CONV,
@@ -3021,6 +3049,7 @@ static s32 *coef_addr_table[TEGRA210_SFC_NUM_RATES][TEGRA210_SFC_NUM_RATES] = {
 		coef_192to32,
 		coef_192to44,
 		coef_192to48,
+		UNSUPP_CONV,
 		coef_192to88,
 		coef_192to96,
 		coef_192to176,
diff --git a/sound/soc/tegra/tegra210_sfc.h b/sound/soc/tegra/tegra210_sfc.h
index 5a6b66e..a4c993d7 100644
--- a/sound/soc/tegra/tegra210_sfc.h
+++ b/sound/soc/tegra/tegra210_sfc.h
@@ -2,7 +2,7 @@
 /*
  * tegra210_sfc.h - Definitions for Tegra210 SFC driver
  *
- * Copyright (c) 2021 NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2021-2023 NVIDIA CORPORATION.  All rights reserved.
  *
  */
 
@@ -47,7 +47,7 @@
 #define TEGRA210_SFC_EN_SHIFT			0
 #define TEGRA210_SFC_EN				(1 << TEGRA210_SFC_EN_SHIFT)
 
-#define TEGRA210_SFC_NUM_RATES 12
+#define TEGRA210_SFC_NUM_RATES 13
 
 /* Fields in TEGRA210_SFC_COEF_RAM */
 #define TEGRA210_SFC_COEF_RAM_EN		BIT(0)
-- 
2.7.4

[PATCH 2/8] ASoC: tegra: Fix AMX byte map

[Sameer Pujar]

From: Sheetal <sheetal@nvidia.com>

Byte mask for channel-1 of stream-1 is not getting enabled and this
causes failures during AMX use cases. The enable bit is not set during
put() callback of byte map mixer control.

This happens because the byte map value 0 matches the initial state
of byte map array and put() callback returns without doing anything.

Fix the put() callback by actually looking at the byte mask array
to identify if any change is needed and update the fields accordingly.
Also update get() callback to return 256 if the byte map is disabled.

Fixes: 8db78ace1ba8 ("ASoC: tegra: Fix kcontrol put callback in AMX")
Cc: stable@vger.kernel.org
Signed-off-by: Sheetal <sheetal@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
Reviewed-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/soc/tegra/tegra210_amx.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/sound/soc/tegra/tegra210_amx.c b/sound/soc/tegra/tegra210_amx.c
index 782a141..1410e8b 100644
--- a/sound/soc/tegra/tegra210_amx.c
+++ b/sound/soc/tegra/tegra210_amx.c
@@ -2,7 +2,7 @@
 //
 // tegra210_amx.c - Tegra210 AMX driver
 //
-// Copyright (c) 2021 NVIDIA CORPORATION.  All rights reserved.
+// Copyright (c) 2021-2023 NVIDIA CORPORATION.  All rights reserved.
 
 #include <linux/clk.h>
 #include <linux/device.h>
@@ -206,7 +206,7 @@ static int tegra210_amx_get_byte_map(struct snd_kcontrol *kcontrol,
 	if (enabled)
 		ucontrol->value.integer.value[0] = bytes_map[reg];
 	else
-		ucontrol->value.integer.value[0] = 0;
+		ucontrol->value.integer.value[0] = 256;
 
 	return 0;
 }
@@ -221,25 +221,19 @@ static int tegra210_amx_put_byte_map(struct snd_kcontrol *kcontrol,
 	unsigned char *bytes_map = (unsigned char *)&amx->map;
 	int reg = mc->reg;
 	int value = ucontrol->value.integer.value[0];
+	unsigned int mask_val = amx->byte_mask[reg / 32];
 
-	if (value == bytes_map[reg])
+	if (value >= 0 && value <= 255)
+		mask_val |= (1 << (reg % 32));
+	else
+		mask_val &= ~(1 << (reg % 32));
+
+	if (mask_val == amx->byte_mask[reg / 32])
 		return 0;
 
-	if (value >= 0 && value <= 255) {
-		/* Update byte map and enable slot */
-		bytes_map[reg] = value;
-		if (reg > 31)
-			amx->byte_mask[1] |= (1 << (reg - 32));
-		else
-			amx->byte_mask[0] |= (1 << reg);
-	} else {
-		/* Reset byte map and disable slot */
-		bytes_map[reg] = 0;
-		if (reg > 31)
-			amx->byte_mask[1] &= ~(1 << (reg - 32));
-		else
-			amx->byte_mask[0] &= ~(1 << reg);
-	}
+	/* Update byte map and slot */
+	bytes_map[reg] = value % 256;
+	amx->byte_mask[reg / 32] = mask_val;
 
 	return 1;
 }
-- 
2.7.4

[PATCH 3/8] ASoC: tegra: Fix ADX byte map

[Sameer Pujar]

From: Sheetal <sheetal@nvidia.com>

Byte mask for channel-1 of stream-1 is not getting enabled and this
causes failures during ADX use cases. The enable bit is not set during
put() callback of byte map mixer control.

This happens because the byte map value 0 matches the initial state
of byte map array and put() callback returns without doing anything.

Fix the put() callback by actually looking at the byte mask array
to identify if any change is needed and update the fields accordingly.
Also update get() callback to return 256 if the byte map is disabled.

Fixes: 3c97881b8c8a ("ASoC: tegra: Fix kcontrol put callback in ADX")
Cc: stable@vger.kernel.org
Signed-off-by: Sheetal <sheetal@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
Reviewed-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/soc/tegra/tegra210_adx.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/sound/soc/tegra/tegra210_adx.c b/sound/soc/tegra/tegra210_adx.c
index bd0b10c..6894b11 100644
--- a/sound/soc/tegra/tegra210_adx.c
+++ b/sound/soc/tegra/tegra210_adx.c
@@ -2,7 +2,7 @@
 //
 // tegra210_adx.c - Tegra210 ADX driver
 //
-// Copyright (c) 2021 NVIDIA CORPORATION.  All rights reserved.
+// Copyright (c) 2021-2023 NVIDIA CORPORATION.  All rights reserved.
 
 #include <linux/clk.h>
 #include <linux/device.h>
@@ -178,7 +178,7 @@ static int tegra210_adx_get_byte_map(struct snd_kcontrol *kcontrol,
 	if (enabled)
 		ucontrol->value.integer.value[0] = bytes_map[mc->reg];
 	else
-		ucontrol->value.integer.value[0] = 0;
+		ucontrol->value.integer.value[0] = 256;
 
 	return 0;
 }
@@ -192,19 +192,19 @@ static int tegra210_adx_put_byte_map(struct snd_kcontrol *kcontrol,
 	int value = ucontrol->value.integer.value[0];
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
+	unsigned int mask_val = adx->byte_mask[mc->reg / 32];
 
-	if (value == bytes_map[mc->reg])
+	if (value >= 0 && value <= 255)
+		mask_val |= (1 << (mc->reg % 32));
+	else
+		mask_val &= ~(1 << (mc->reg % 32));
+
+	if (mask_val == adx->byte_mask[mc->reg / 32])
 		return 0;
 
-	if (value >= 0 && value <= 255) {
-		/* update byte map and enable slot */
-		bytes_map[mc->reg] = value;
-		adx->byte_mask[mc->reg / 32] |= (1 << (mc->reg % 32));
-	} else {
-		/* reset byte map and disable slot */
-		bytes_map[mc->reg] = 0;
-		adx->byte_mask[mc->reg / 32] &= ~(1 << (mc->reg % 32));
-	}
+	/* Update byte map and slot */
+	bytes_map[mc->reg] = value % 256;
+	adx->byte_mask[mc->reg / 32] = mask_val;
 
 	return 1;
 }
-- 
2.7.4

[PATCH 4/8] ASoC: rt5640: Fix sleep in atomic context

[Sameer Pujar]

Following prints are observed while testing audio on Jetson AGX Orin which
has onboard RT5640 audio codec:

  BUG: sleeping function called from invalid context at kernel/workqueue.c:3027
  in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/0
  preempt_count: 10001, expected: 0
  RCU nest depth: 0, expected: 0
  ------------[ cut here ]------------
  WARNING: CPU: 0 PID: 0 at kernel/irq/handle.c:159 __handle_irq_event_percpu+0x1e0/0x270
  ---[ end trace ad1c64905aac14a6 ]-

The IRQ handler rt5640_irq() runs in interrupt context and can sleep
during cancel_delayed_work_sync().

Fix this by running IRQ handler, rt5640_irq(), in thread context.
Hence replace request_irq() calls with devm_request_threaded_irq().

Fixes: 051dade34695 ("ASoC: rt5640: Fix the wrong state of JD1 and JD2")
Cc: stable@vger.kernel.org
Cc: Oder Chiou <oder_chiou@realtek.com>
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/soc/codecs/rt5640.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index c7d2f31..a54d2bd 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -2552,9 +2552,11 @@ static void rt5640_enable_jack_detect(struct snd_soc_component *component,
 		rt5640->jd_gpio = jack_data->jd_gpio;
 		rt5640->jd_gpio_irq = gpiod_to_irq(rt5640->jd_gpio);
 
-		ret = request_irq(rt5640->jd_gpio_irq, rt5640_jd_gpio_irq,
-				  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-				  "rt5640-jd-gpio", rt5640);
+		ret = devm_request_threaded_irq(component->dev,
+				rt5640->jd_gpio_irq,
+				NULL, rt5640_jd_gpio_irq,
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				"rt5640-jd-gpio", rt5640);
 		if (ret) {
 			dev_warn(component->dev, "Failed to request jd GPIO IRQ %d: %d\n",
 				 rt5640->jd_gpio_irq, ret);
@@ -2567,9 +2569,10 @@ static void rt5640_enable_jack_detect(struct snd_soc_component *component,
 	if (jack_data && jack_data->use_platform_clock)
 		rt5640->use_platform_clock = jack_data->use_platform_clock;
 
-	ret = request_irq(rt5640->irq, rt5640_irq,
-			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-			  "rt5640", rt5640);
+	ret = devm_request_threaded_irq(component->dev, rt5640->irq,
+			NULL, rt5640_irq,
+			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+			"rt5640", rt5640);
 	if (ret) {
 		dev_warn(component->dev, "Failed to reguest IRQ %d: %d\n", rt5640->irq, ret);
 		rt5640_disable_jack_detect(component);
@@ -2622,8 +2625,9 @@ static void rt5640_enable_hda_jack_detect(
 
 	rt5640->jack = jack;
 
-	ret = request_irq(rt5640->irq, rt5640_irq,
-			  IRQF_TRIGGER_RISING | IRQF_ONESHOT, "rt5640", rt5640);
+	ret = devm_request_threaded_irq(component->dev, rt5640->irq,
+			NULL, rt5640_irq, IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+			"rt5640", rt5640);
 	if (ret) {
 		dev_warn(component->dev, "Failed to reguest IRQ %d: %d\n", rt5640->irq, ret);
 		rt5640->irq = -ENXIO;
-- 
2.7.4

[PATCH 5/8] ASoC: tegra: Use normal system sleep for ASRC

[Sameer Pujar]

Align with other AHUB module drivers and use normal system
sleep for ASRC as well.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/soc/tegra/tegra186_asrc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/tegra/tegra186_asrc.c b/sound/soc/tegra/tegra186_asrc.c
index e016a6a..208e2fc 100644
--- a/sound/soc/tegra/tegra186_asrc.c
+++ b/sound/soc/tegra/tegra186_asrc.c
@@ -1024,8 +1024,8 @@ static void tegra186_asrc_platform_remove(struct platform_device *pdev)
 static const struct dev_pm_ops tegra186_asrc_pm_ops = {
 	SET_RUNTIME_PM_OPS(tegra186_asrc_runtime_suspend,
 			   tegra186_asrc_runtime_resume, NULL)
-	SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
-				     pm_runtime_force_resume)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
 };
 
 static struct platform_driver tegra186_asrc_driver = {
-- 
2.7.4

[PATCH 6/8] ASoC: tegra: Remove stale comments in AHUB

[Sameer Pujar]

Remove stale comments in AHUB driver which is related to DAPM
widgets and routes. This is misleading otherwise.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/soc/tegra/tegra210_ahub.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/sound/soc/tegra/tegra210_ahub.c b/sound/soc/tegra/tegra210_ahub.c
index 8c00c09..3f114a2 100644
--- a/sound/soc/tegra/tegra210_ahub.c
+++ b/sound/soc/tegra/tegra210_ahub.c
@@ -712,11 +712,6 @@ MUX_ENUM_CTRL_DECL_234(t234_asrc15_tx, 0x68);
 MUX_ENUM_CTRL_DECL_234(t234_asrc16_tx, 0x69);
 MUX_ENUM_CTRL_DECL_234(t234_asrc17_tx, 0x6a);
 
-/*
- * The number of entries in, and order of, this array is closely tied to the
- * calculation of tegra210_ahub_codec.num_dapm_widgets near the end of
- * tegra210_ahub_probe()
- */
 static const struct snd_soc_dapm_widget tegra210_ahub_widgets[] = {
 	WIDGETS("ADMAIF1", t210_admaif1_tx),
 	WIDGETS("ADMAIF2", t210_admaif2_tx),
@@ -1092,11 +1087,6 @@ static const struct snd_soc_dapm_widget tegra234_ahub_widgets[] = {
 	{ name " XBAR-Capture",		NULL,	name " XBAR-TX" },      \
 	{ name " Capture",		NULL,	name " XBAR-Capture" },
 
-/*
- * The number of entries in, and order of, this array is closely tied to the
- * calculation of tegra210_ahub_codec.num_dapm_routes near the end of
- * tegra210_ahub_probe()
- */
 static const struct snd_soc_dapm_route tegra210_ahub_routes[] = {
 	TEGRA_FE_ROUTES("ADMAIF1")
 	TEGRA_FE_ROUTES("ADMAIF2")
-- 
2.7.4

[PATCH 7/8] arm64: tegra: Update AHUB clock parent and rate on Tegra234

[Sameer Pujar]

From: Sheetal <sheetal@nvidia.com>

I2S data sanity tests fail beyond a bit clock frequency of 6.144MHz.
This happens because the AHUB clock rate is too low and it shows
9.83MHz on boot.

The maximum rate of PLLA_OUT0 is 49.152MHz and is used to serve I/O
clocks. It is recommended that AHUB clock operates higher than this.
Thus fix this by using PLLP_OUT0 as parent clock for AHUB instead of
PLLA_OUT0 and fix the rate to 81.6MHz.

Fixes: dc94a94daa39 ("arm64: tegra: Add audio devices on Tegra234")
Cc: stable@vger.kernel.org
Signed-off-by: Sheetal <sheetal@nvidia.com>
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index f4974e8..0f12a8de 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -180,7 +180,8 @@
 				clocks = <&bpmp TEGRA234_CLK_AHUB>;
 				clock-names = "ahub";
 				assigned-clocks = <&bpmp TEGRA234_CLK_AHUB>;
-				assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLA_OUT0>;
+				assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLP_OUT0>;
+				assigned-clock-rates = <81600000>;
 				status = "disabled";
 
 				#address-cells = <2>;
-- 
2.7.4

[PATCH 8/8] arm64: tegra: Update AHUB clock parent and rate

[Sameer Pujar]

I2S data sanity test failures are seen at lower AHUB clock rates
on Tegra234. The Tegra194 uses the same clock relationship for AHUB
and it is likely that similar issues would be seen. Thus update the
AHUB clock parent and rates here as well for Tegra194, Tegra186
and Tegra210.

Fixes: 177208f7b06d ("arm64: tegra: Add DT binding for AHUB components")
Cc: stable@vger.kernel.org
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 3 ++-
 arch/arm64/boot/dts/nvidia/tegra194.dtsi | 3 ++-
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 7e4c496f..2b3bb5d 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -135,7 +135,8 @@
 			clocks = <&bpmp TEGRA186_CLK_AHUB>;
 			clock-names = "ahub";
 			assigned-clocks = <&bpmp TEGRA186_CLK_AHUB>;
-			assigned-clock-parents = <&bpmp TEGRA186_CLK_PLL_A_OUT0>;
+			assigned-clock-parents = <&bpmp TEGRA186_CLK_PLLP_OUT0>;
+			assigned-clock-rates = <81600000>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges = <0x02900800 0x02900800 0x11800>;
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 154fc8c..33f92b7 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -231,7 +231,8 @@
 				clocks = <&bpmp TEGRA194_CLK_AHUB>;
 				clock-names = "ahub";
 				assigned-clocks = <&bpmp TEGRA194_CLK_AHUB>;
-				assigned-clock-parents = <&bpmp TEGRA194_CLK_PLLA_OUT0>;
+				assigned-clock-parents = <&bpmp TEGRA194_CLK_PLLP_OUT0>;
+				assigned-clock-rates = <81600000>;
 				status = "disabled";
 
 				#address-cells = <2>;
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 617583f..e7b4e30 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -1386,7 +1386,8 @@
 			clocks = <&tegra_car TEGRA210_CLK_D_AUDIO>;
 			clock-names = "ahub";
 			assigned-clocks = <&tegra_car TEGRA210_CLK_D_AUDIO>;
-			assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_A_OUT0>;
+			assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_P>;
+			assigned-clock-rates = <81600000>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges = <0x702d0000 0x702d0000 0x0000e400>;
-- 
2.7.4

Re: [PATCH 2/8] ASoC: tegra: Fix AMX byte map

[Mark Brown]

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

On Thu, Jun 22, 2023 at 05:04:10PM +0530, Sameer Pujar wrote:
> From: Sheetal <sheetal@nvidia.com>
> 
> Byte mask for channel-1 of stream-1 is not getting enabled and this
> causes failures during AMX use cases. The enable bit is not set during
> put() callback of byte map mixer control.
> 
> This happens because the byte map value 0 matches the initial state
> of byte map array and put() callback returns without doing anything.
> 
> Fix the put() callback by actually looking at the byte mask array
> to identify if any change is needed and update the fields accordingly.

I'm not quite sure I follow the logic here - I'd have expected this to
mean that there's a bootstrapping issue and that we should be doing some
more initialisation during startup such that the existing code which
checks if there is a change will be doing the right thing?

> Also update get() callback to return 256 if the byte map is disabled.

This will be a user visible change.  It's not clear to me why it's
needed - it seems like it's a hack to push users to do an update in the
case where they want to use channel 1 stream 1?

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

Re: [PATCH 4/8] ASoC: rt5640: Fix sleep in atomic context

[Mark Brown]

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

On Thu, Jun 22, 2023 at 05:04:12PM +0530, Sameer Pujar wrote:

> The IRQ handler rt5640_irq() runs in interrupt context and can sleep
> during cancel_delayed_work_sync().

> Fix this by running IRQ handler, rt5640_irq(), in thread context.
> Hence replace request_irq() calls with devm_request_threaded_irq().

> -		ret = request_irq(rt5640->jd_gpio_irq, rt5640_jd_gpio_irq,
> -				  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> -				  "rt5640-jd-gpio", rt5640);
> +		ret = devm_request_threaded_irq(component->dev,
> +				rt5640->jd_gpio_irq,
> +				NULL, rt5640_jd_gpio_irq,
> +				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +				"rt5640-jd-gpio", rt5640);

This is rt5640_jd_gpio_irq() which just does a queue_delayed_work() not
a cancel.  Why is it being changed?

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

Re: [PATCH 7/8] arm64: tegra: Update AHUB clock parent and rate on Tegra234

[Mark Brown]

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

On Thu, Jun 22, 2023 at 05:04:15PM +0530, Sameer Pujar wrote:
> From: Sheetal <sheetal@nvidia.com>
> 
> I2S data sanity tests fail beyond a bit clock frequency of 6.144MHz.
> This happens because the AHUB clock rate is too low and it shows
> 9.83MHz on boot.
> 
> The maximum rate of PLLA_OUT0 is 49.152MHz and is used to serve I/O
> clocks. It is recommended that AHUB clock operates higher than this.
> Thus fix this by using PLLP_OUT0 as parent clock for AHUB instead of
> PLLA_OUT0 and fix the rate to 81.6MHz.
> 
> Fixes: dc94a94daa39 ("arm64: tegra: Add audio devices on Tegra234")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sheetal <sheetal@nvidia.com>

Fixes should come before cleanups in a patch series to ensure that they
can be applied and sent as fixes without dependencies on non-fixes.

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

Re: (subset) [PATCH 0/8] Few audio fixes on Tegra platforms

[Mark Brown]

On Thu, 22 Jun 2023 17:04:08 +0530, Sameer Pujar wrote:
> This series fixes some of the issues which were observed during an attempt to
> enhance automated test coverage on Jetson AGX Orin. Below is a short summary
> of the issues and fixes:
> 
>   * Sample rate coversion failures above 48kHz.
>   * AMX and ADX test cases failures due to incorrect byte mask.
>   * Atomic sleep in RT5640 codec which is present on Jetson AGX Orin.
>   * AHUB clock fixes on Tegra234 and previous chips.
>   * Minor cleanups in ASRC and AHUB driver.
> 
> [...]

Applied to

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

Thanks!

[1/8] ASoC: tegra: Fix SFC conversion for few rates
      (no commit info)
[5/8] ASoC: tegra: Use normal system sleep for ASRC
      commit: 2cc41db71a434844ca97b6e30c9a30a2464a996e
[6/8] ASoC: tegra: Remove stale comments in AHUB
      commit: f47d43283a4233528683deaaba95f0ee2cfe862d

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

Re: [PATCH 7/8] arm64: tegra: Update AHUB clock parent and rate on Tegra234

[Sameer Pujar]

On 22-06-2023 17:43, Mark Brown wrote:
> On Thu, Jun 22, 2023 at 05:04:15PM +0530, Sameer Pujar wrote:
>> From: Sheetal <sheetal@nvidia.com>
>>
>> I2S data sanity tests fail beyond a bit clock frequency of 6.144MHz.
>> This happens because the AHUB clock rate is too low and it shows
>> 9.83MHz on boot.
>>
>> The maximum rate of PLLA_OUT0 is 49.152MHz and is used to serve I/O
>> clocks. It is recommended that AHUB clock operates higher than this.
>> Thus fix this by using PLLP_OUT0 as parent clock for AHUB instead of
>> PLLA_OUT0 and fix the rate to 81.6MHz.
>>
>> Fixes: dc94a94daa39 ("arm64: tegra: Add audio devices on Tegra234")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Sheetal <sheetal@nvidia.com>
> Fixes should come before cleanups in a patch series to ensure that they
> can be applied and sent as fixes without dependencies on non-fixes.

I sorted the series based on the subsystem. Will make sure to put 
'fixes' patch always first. Thanks.

Re: [PATCH 4/8] ASoC: rt5640: Fix sleep in atomic context

[Sameer Pujar]

On 22-06-2023 17:42, Mark Brown wrote:
> On Thu, Jun 22, 2023 at 05:04:12PM +0530, Sameer Pujar wrote:
>
>> The IRQ handler rt5640_irq() runs in interrupt context and can sleep
>> during cancel_delayed_work_sync().
>> Fix this by running IRQ handler, rt5640_irq(), in thread context.
>> Hence replace request_irq() calls with devm_request_threaded_irq().
>> -		ret = request_irq(rt5640->jd_gpio_irq, rt5640_jd_gpio_irq,
>> -				  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
>> -				  "rt5640-jd-gpio", rt5640);
>> +		ret = devm_request_threaded_irq(component->dev,
>> +				rt5640->jd_gpio_irq,
>> +				NULL, rt5640_jd_gpio_irq,
>> +				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
>> +				"rt5640-jd-gpio", rt5640);
> This is rt5640_jd_gpio_irq() which just does a queue_delayed_work() not
> a cancel.  Why is it being changed?

Will drop this part in v2. Thanks.

Re: [PATCH 2/8] ASoC: tegra: Fix AMX byte map

[Sameer Pujar]

On 22-06-2023 17:37, Mark Brown wrote:
> On Thu, Jun 22, 2023 at 05:04:10PM +0530, Sameer Pujar wrote:
>> From: Sheetal <sheetal@nvidia.com>
>>
>> Byte mask for channel-1 of stream-1 is not getting enabled and this
>> causes failures during AMX use cases. The enable bit is not set during
>> put() callback of byte map mixer control.
>>
>> This happens because the byte map value 0 matches the initial state
>> of byte map array and put() callback returns without doing anything.
>>
>> Fix the put() callback by actually looking at the byte mask array
>> to identify if any change is needed and update the fields accordingly.
> I'm not quite sure I follow the logic here - I'd have expected this to
> mean that there's a bootstrapping issue and that we should be doing some
> more initialisation during startup such that the existing code which
> checks if there is a change will be doing the right thing?
The issue can happen in subsequent cycles as well if once the user 
disables the byte map by putting 256. It happens because of following 
reason where 256 value is reset to 0 since the byte map array is tightly 
packed and it can't store 256 value.

static int tegra210_amx_put_byte_map() {
         ...
         if (value >= 0 && value <= 255)
             mask_val |= (1 << (reg % 32));
         else
             mask_val &= ~(1 << (reg % 32));

         if (mask_val == amx->byte_mask[reg / 32])
             return 0;

         /* Update byte map and slot */
==>     bytes_map[reg] = value % 256;
         amx->byte_mask[reg / 32] = mask_val;

         return 1;
}

>> Also update get() callback to return 256 if the byte map is disabled.
> This will be a user visible change.  It's not clear to me why it's
> needed - it seems like it's a hack to push users to do an update in the
> case where they want to use channel 1 stream 1?

Though it looks like 256 value is forced, but actually the user sees 
whatever value is set before. The 256 value storage is linked to byte 
mask value.

I must admit that this is not easily readable. If you suggest to 
simplify this, I can check if storage space increase for byte map value 
can make it more readable. Thanks for your feedback.

Re: [PATCH 2/8] ASoC: tegra: Fix AMX byte map

[Mark Brown]

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

On Fri, Jun 23, 2023 at 11:09:32AM +0530, Sameer Pujar wrote:
> On 22-06-2023 17:37, Mark Brown wrote:
> > On Thu, Jun 22, 2023 at 05:04:10PM +0530, Sameer Pujar wrote:

> > > Byte mask for channel-1 of stream-1 is not getting enabled and this
> > > causes failures during AMX use cases. The enable bit is not set during
> > > put() callback of byte map mixer control.

> > > This happens because the byte map value 0 matches the initial state
> > > of byte map array and put() callback returns without doing anything.

> > > Fix the put() callback by actually looking at the byte mask array
> > > to identify if any change is needed and update the fields accordingly.

> > I'm not quite sure I follow the logic here - I'd have expected this to
> > mean that there's a bootstrapping issue and that we should be doing some
> > more initialisation during startup such that the existing code which
> > checks if there is a change will be doing the right thing?

> The issue can happen in subsequent cycles as well if once the user disables
> the byte map by putting 256. It happens because of following reason where
> 256 value is reset to 0 since the byte map array is tightly packed and it
> can't store 256 value.

...

> > > Also update get() callback to return 256 if the byte map is disabled.
> > This will be a user visible change.  It's not clear to me why it's
> > needed - it seems like it's a hack to push users to do an update in the
> > case where they want to use channel 1 stream 1?

> Though it looks like 256 value is forced, but actually the user sees
> whatever value is set before. The 256 value storage is linked to byte mask
> value.

> I must admit that this is not easily readable. If you suggest to simplify
> this, I can check if storage space increase for byte map value can make it
> more readable. Thanks for your feedback.

This could definitely use more clarification I think.  It's not obvious
that storing 256 won't actually hold (and that should trigger a
complaint from mixer-test if that's what happens).

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

Re: [PATCH 2/8] ASoC: tegra: Fix AMX byte map

[Sameer Pujar]

On 23-06-2023 15:45, Mark Brown wrote:
> On Fri, Jun 23, 2023 at 11:09:32AM +0530, Sameer Pujar wrote:
>> On 22-06-2023 17:37, Mark Brown wrote:
>>> On Thu, Jun 22, 2023 at 05:04:10PM +0530, Sameer Pujar wrote:
>>>> Byte mask for channel-1 of stream-1 is not getting enabled and this
>>>> causes failures during AMX use cases. The enable bit is not set during
>>>> put() callback of byte map mixer control.
>>>> This happens because the byte map value 0 matches the initial state
>>>> of byte map array and put() callback returns without doing anything.
>>>> Fix the put() callback by actually looking at the byte mask array
>>>> to identify if any change is needed and update the fields accordingly.
>>> I'm not quite sure I follow the logic here - I'd have expected this to
>>> mean that there's a bootstrapping issue and that we should be doing some
>>> more initialisation during startup such that the existing code which
>>> checks if there is a change will be doing the right thing?
>> The issue can happen in subsequent cycles as well if once the user disables
>> the byte map by putting 256. It happens because of following reason where
>> 256 value is reset to 0 since the byte map array is tightly packed and it
>> can't store 256 value.
> ...
>
>>>> Also update get() callback to return 256 if the byte map is disabled.
>>> This will be a user visible change.  It's not clear to me why it's
>>> needed - it seems like it's a hack to push users to do an update in the
>>> case where they want to use channel 1 stream 1?
>> Though it looks like 256 value is forced, but actually the user sees
>> whatever value is set before. The 256 value storage is linked to byte mask
>> value.
>> I must admit that this is not easily readable. If you suggest to simplify
>> this, I can check if storage space increase for byte map value can make it
>> more readable. Thanks for your feedback.
> This could definitely use more clarification I think.  It's not obvious
> that storing 256 won't actually hold (and that should trigger a
> complaint from mixer-test if that's what happens).

OK. I will provide more clarifications in the commit message and the 
driver for the existing failure. Will put a TODO item in the driver to 
improve the logic and make it more readable.