[PATCH v2 1/4] ASoC: rt721-sdca: handle HID button read errors

[PATCH v2 1/4] ASoC: rt721-sdca: handle HID button read errors

[Avi Weiss]

rt_sdca_button_detect() returns zero when a regmap read fails,
so RT721 treats the error as a valid no-button result.

Return the original error and skip button reporting when a read
fails. Still report jack changes, and clear button state if the
headset is removed.

Fixes: bbca8e7050e0 ("ASoC: rt-sdw-common: Common functions for Realtek soundwire driver")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
---
Changes in v2:
- report jack state on button read failures even when SDCA_0 is clear, fixing the missed unplug identified during review of v1
- document negative error returns from rt_sdca_button_detect()

 sound/soc/codecs/rt-sdw-common.c |  6 +++++-
 sound/soc/codecs/rt721-sdca.c    | 11 +++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt-sdw-common.c b/sound/soc/codecs/rt-sdw-common.c
index ad61943ce75ff..2b24d6ac8f143 100644
--- a/sound/soc/codecs/rt-sdw-common.c
+++ b/sound/soc/codecs/rt-sdw-common.c
@@ -187,6 +187,7 @@ EXPORT_SYMBOL_GPL(rt_sdca_headset_detect);
  *
  * A button type will be returned regarding to buffer,
  * it returns zero if buffer cannot be recognized.
+ * A negative errno is returned if a register read fails.
  */
 int rt_sdca_button_detect(struct regmap *map, unsigned int entity_id,
 	unsigned int hid_buf_addr, unsigned int hid_id)
@@ -199,7 +200,7 @@ int rt_sdca_button_detect(struct regmap *map, unsigned int entity_id,
 	ret = regmap_read(map, SDW_SDCA_CTL(SDCA_NUM_HID, entity_id,
 			RT_SDCA_CTL_HIDTX_CURRENT_OWNER, 0), &owner);
 	if (ret < 0)
-		return 0;
+		return ret;
 
 	/* if owner is device then there is no button event from device */
 	if (owner == 1)
@@ -229,6 +230,9 @@ int rt_sdca_button_detect(struct regmap *map, unsigned int entity_id,
 			SDW_SDCA_CTL(SDCA_NUM_HID, entity_id,
 				RT_SDCA_CTL_HIDTX_CURRENT_OWNER, 0), 0x01);
 
+	if (ret < 0)
+		return ret;
+
 	return btn_type;
 }
 EXPORT_SYMBOL_GPL(rt_sdca_button_detect);
diff --git a/sound/soc/codecs/rt721-sdca.c b/sound/soc/codecs/rt721-sdca.c
index 9f567caecb70a..82ef8245474cb 100644
--- a/sound/soc/codecs/rt721-sdca.c
+++ b/sound/soc/codecs/rt721-sdca.c
@@ -248,6 +248,17 @@ static void rt721_sdca_jack_detect_handler(struct work_struct *work)
 					RT721_SDCA_ENT_HID01, RT721_BUF_ADDR_HID1,
 					RT721_SDCA_HID_ID);
 
+	if (btn_type < 0) {
+		unsigned int mask = SND_JACK_HEADSET;
+
+		if (rt721->jack_type != SND_JACK_HEADSET)
+			mask |= SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+				SND_JACK_BTN_2 | SND_JACK_BTN_3;
+
+		snd_soc_jack_report(rt721->hs_jack, rt721->jack_type, mask);
+		return;
+	}
+
 	if (rt721->jack_type == 0)
 		btn_type = 0;
 

[PATCH v2 2/4] ASoC: rt711-sdca: handle HID button read errors

[Avi Weiss]

rt711_sdca_button_detect() returns zero when a regmap read fails,
so the jack handler cannot distinguish the error from a valid
no-button result.

Return the original error and make the button result signed.
Skip button reporting on read failure, but still report jack
changes and clear button state when the headset is removed.

Fixes: 7ad4d237e7c4 ("ASoC: rt711-sdca: Add RT711 SDCA vendor-specific driver")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
---
Changes in v2:
- report jack state on button read failures regardless of SDCA_0, preserving the original jack reporting behavior

 sound/soc/codecs/rt711-sdca.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/rt711-sdca.c b/sound/soc/codecs/rt711-sdca.c
index 9781f289f8adc..f6c8377c51ab5 100644
--- a/sound/soc/codecs/rt711-sdca.c
+++ b/sound/soc/codecs/rt711-sdca.c
@@ -184,10 +184,10 @@ static int rt711_sdca_calibration(struct rt711_sdca_priv *rt711)
 	return ret;
 }
 
-static unsigned int rt711_sdca_button_detect(struct rt711_sdca_priv *rt711)
+static int rt711_sdca_button_detect(struct rt711_sdca_priv *rt711)
 {
-	unsigned int btn_type = 0, offset, idx, val, owner;
-	int ret;
+	unsigned int offset, idx, val, owner;
+	int btn_type = 0, ret;
 	unsigned char buf[3];
 
 	/* get current UMP message owner */
@@ -195,7 +195,7 @@ static unsigned int rt711_sdca_button_detect(struct rt711_sdca_priv *rt711)
 		SDW_SDCA_CTL(FUNC_NUM_HID, RT711_SDCA_ENT_HID01, RT711_SDCA_CTL_HIDTX_CURRENT_OWNER, 0),
 		&owner);
 	if (ret < 0)
-		return 0;
+		return ret;
 
 	/* if owner is device then there is no button event from device */
 	if (owner == 1)
@@ -259,6 +259,9 @@ static unsigned int rt711_sdca_button_detect(struct rt711_sdca_priv *rt711)
 			SDW_SDCA_CTL(FUNC_NUM_HID, RT711_SDCA_ENT_HID01,
 				RT711_SDCA_CTL_HIDTX_SET_OWNER_TO_DEVICE, 0), 0x01);
 
+	if (ret < 0)
+		return ret;
+
 	return btn_type;
 }
 
@@ -330,6 +333,17 @@ static void rt711_sdca_jack_detect_handler(struct work_struct *work)
 	if (rt711->scp_sdca_stat2 & SDW_SCP_SDCA_INT_SDCA_8)
 		btn_type = rt711_sdca_button_detect(rt711);
 
+	if (btn_type < 0) {
+		unsigned int mask = SND_JACK_HEADSET;
+
+		if (rt711->jack_type != SND_JACK_HEADSET)
+			mask |= SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+				SND_JACK_BTN_2 | SND_JACK_BTN_3;
+
+		snd_soc_jack_report(rt711->hs_jack, rt711->jack_type, mask);
+		return;
+	}
+
 	if (rt711->jack_type == 0)
 		btn_type = 0;
 

Re: [PATCH v2 2/4] ASoC: rt711-sdca: handle HID button read errors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: rt711-sdca: no-op error path in rt711_sdca_jack_detect_handler()
--

commit 54e5617fc05f75e04b8c64c9ddcc952bcf29b404
Author: Avi Weiss <thnkslprpt@gmail.com>

ASoC: rt711-sdca: handle HID button read errors

This commit changes rt711_sdca_button_detect() to return negative error
codes on regmap_read() failures instead of zero, and updates
rt711_sdca_jack_detect_handler() to handle negative btn_type values.

> diff --git a/sound/soc/codecs/rt711-sdca.c b/sound/soc/codecs/rt711-sdca.c
> index 9781f289f8adc..f6c8377c51ab5 100644
> --- a/sound/soc/codecs/rt711-sdca.c
> +++ b/sound/soc/codecs/rt711-sdca.c

[ ... ]

> @@ -330,6 +333,17 @@ static void rt711_sdca_jack_detect_handler(struct work_struct *work)
>  	if (rt711->scp_sdca_stat2 & SDW_SCP_SDCA_INT_SDCA_8)
>  		btn_type = rt711_sdca_button_detect(rt711);
>  
> +	if (btn_type < 0) {
> +		unsigned int mask = SND_JACK_HEADSET;
> +
> +		if (rt711->jack_type != SND_JACK_HEADSET)
> +			mask |= SND_JACK_BTN_0 | SND_JACK_BTN_1 |
> +				SND_JACK_BTN_2 | SND_JACK_BTN_3;
> +
> +		snd_soc_jack_report(rt711->hs_jack, rt711->jack_type, mask);
> +		return;
> +	}

[Severity: Medium]
Does this btn_type < 0 branch actually change the jack or button reporting
behavior when regmap_read() fails in rt711_sdca_button_detect()?

In sound/soc/codecs/rt711-sdca.c, both rt711_sdca_jack_detect_handler() and
rt711_sdca_btn_check_handler() immediately follow every non-zero button press
report with a synchronous release report that clears SND_JACK_BTN_0..3:

    snd_soc_jack_report(rt711->hs_jack, rt711->jack_type | btn_type,
            SND_JACK_HEADSET |
            SND_JACK_BTN_0 | SND_JACK_BTN_1 |
            SND_JACK_BTN_2 | SND_JACK_BTN_3);

    if (btn_type) {
        /* button released */
        snd_soc_jack_report(rt711->hs_jack, rt711->jack_type,
            SND_JACK_HEADSET |
            SND_JACK_BTN_0 | SND_JACK_BTN_1 |
            SND_JACK_BTN_2 | SND_JACK_BTN_3);

Because SND_JACK_BTN_0..3 are never left set in rt711->hs_jack->status
across work invocations, and snd_soc_jack_report() -> snd_jack_report()
reports the full jack->status for all keys and switches in jack->type
regardless of mask, calling:

    snd_soc_jack_report(rt711->hs_jack, rt711->jack_type, mask);

produces the exact same hs_jack->status value and input notifications as the
normal btn_type == 0 path below (aside from skipping the dev_dbg() messages).

Additionally, because snd_soc_jack_report() is called here even when
SDW_SCP_SDCA_INT_SDCA_0 is not set in rt711->scp_sdca_stat1, a button-only
interrupt where rt711_sdca_button_detect() fails will still perform a full
jack and button report rather than skipping button reporting.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101110056.423353-5-thnkslprpt@gmail.com?part=2

[PATCH v2 3/4] ASoC: rt712-sdca: handle HID button read errors

[Avi Weiss]

rt712_sdca_button_detect() returns zero when a regmap read fails,
so the jack handler cannot distinguish the error from a valid
no-button result.

Return the original error and make the button result signed.
Skip button reporting on read failure, but still report jack
changes and clear button state when the headset is removed.

Fixes: 6c39710da75c ("ASoC: rt712-sdca: Add RT712 SDCA driver for Jack and Amp topology")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
---
Changes in v2:
- report jack state on button read failures regardless of SDCA_0, preserving the original jack reporting behavior

 sound/soc/codecs/rt712-sdca.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/rt712-sdca.c b/sound/soc/codecs/rt712-sdca.c
index 20961e5760c65..08e08ec96bc50 100644
--- a/sound/soc/codecs/rt712-sdca.c
+++ b/sound/soc/codecs/rt712-sdca.c
@@ -335,10 +335,10 @@ static int rt712_sdca_calibration(struct rt712_sdca_priv *rt712)
 	return ret;
 }
 
-static unsigned int rt712_sdca_button_detect(struct rt712_sdca_priv *rt712)
+static int rt712_sdca_button_detect(struct rt712_sdca_priv *rt712)
 {
-	unsigned int btn_type = 0, offset, idx, val, owner;
-	int ret;
+	unsigned int offset, idx, val, owner;
+	int btn_type = 0, ret;
 	unsigned char buf[3];
 
 	/* get current UMP message owner */
@@ -346,7 +346,7 @@ static unsigned int rt712_sdca_button_detect(struct rt712_sdca_priv *rt712)
 		SDW_SDCA_CTL(FUNC_NUM_HID, RT712_SDCA_ENT_HID01, RT712_SDCA_CTL_HIDTX_CURRENT_OWNER, 0),
 		&owner);
 	if (ret < 0)
-		return 0;
+		return ret;
 
 	/* if owner is device then there is no button event from device */
 	if (owner == 1)
@@ -416,6 +416,9 @@ static unsigned int rt712_sdca_button_detect(struct rt712_sdca_priv *rt712)
 					RT712_SDCA_CTL_HIDTX_CURRENT_OWNER, 0), 0x01);
 	}
 
+	if (ret < 0)
+		return ret;
+
 	return btn_type;
 }
 
@@ -485,6 +488,17 @@ static void rt712_sdca_jack_detect_handler(struct work_struct *work)
 	if (rt712->scp_sdca_stat2 & SDW_SCP_SDCA_INT_SDCA_8)
 		btn_type = rt712_sdca_button_detect(rt712);
 
+	if (btn_type < 0) {
+		unsigned int mask = SND_JACK_HEADSET;
+
+		if (rt712->jack_type != SND_JACK_HEADSET)
+			mask |= SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+				SND_JACK_BTN_2 | SND_JACK_BTN_3;
+
+		snd_soc_jack_report(rt712->hs_jack, rt712->jack_type, mask);
+		return;
+	}
+
 	if (rt712->jack_type == 0)
 		btn_type = 0;
 

[PATCH v2 4/4] ASoC: rt722-sdca: handle HID button read errors

[Avi Weiss]

rt722_sdca_button_detect() returns zero when a regmap read fails,
so the jack handler cannot distinguish the error from a valid
no-button result.

Return the original error and make the button result signed.
Skip button reporting on read failure, but still report jack
changes and clear button state when the headset is removed.

Fixes: 7f5d6036ca00 ("ASoC: rt722-sdca: Add RT722 SDCA driver")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
---
Changes in v2:
- report jack state on button read failures regardless of SDCA_0, preserving the original jack reporting behavior

 sound/soc/codecs/rt722-sdca.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/rt722-sdca.c b/sound/soc/codecs/rt722-sdca.c
index 1dad8012d904e..2d47420c0505b 100644
--- a/sound/soc/codecs/rt722-sdca.c
+++ b/sound/soc/codecs/rt722-sdca.c
@@ -95,10 +95,10 @@ static int rt722_sdca_btn_type(unsigned char *buffer)
 	return 0;
 }
 
-static unsigned int rt722_sdca_button_detect(struct rt722_sdca_priv *rt722)
+static int rt722_sdca_button_detect(struct rt722_sdca_priv *rt722)
 {
-	unsigned int btn_type = 0, offset, idx, val, owner;
-	int ret;
+	unsigned int offset, idx, val, owner;
+	int btn_type = 0, ret;
 	unsigned char buf[3];
 
 	/* get current UMP message owner */
@@ -106,7 +106,7 @@ static unsigned int rt722_sdca_button_detect(struct rt722_sdca_priv *rt722)
 		SDW_SDCA_CTL(FUNC_NUM_HID, RT722_SDCA_ENT_HID01,
 			RT722_SDCA_CTL_HIDTX_CURRENT_OWNER, 0), &owner);
 	if (ret < 0)
-		return 0;
+		return ret;
 
 	/* if owner is device then there is no button event from device */
 	if (owner == 1)
@@ -138,6 +138,9 @@ static unsigned int rt722_sdca_button_detect(struct rt722_sdca_priv *rt722)
 			SDW_SDCA_CTL(FUNC_NUM_HID, RT722_SDCA_ENT_HID01,
 				RT722_SDCA_CTL_HIDTX_CURRENT_OWNER, 0), 0x01);
 
+	if (ret < 0)
+		return ret;
+
 	return btn_type;
 }
 
@@ -207,6 +210,17 @@ static void rt722_sdca_jack_detect_handler(struct work_struct *work)
 	if (rt722->scp_sdca_stat2 & SDW_SCP_SDCA_INT_SDCA_8)
 		btn_type = rt722_sdca_button_detect(rt722);
 
+	if (btn_type < 0) {
+		unsigned int mask = SND_JACK_HEADSET;
+
+		if (rt722->jack_type != SND_JACK_HEADSET)
+			mask |= SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+				SND_JACK_BTN_2 | SND_JACK_BTN_3;
+
+		snd_soc_jack_report(rt722->hs_jack, rt722->jack_type, mask);
+		return;
+	}
+
 	if (rt722->jack_type == 0)
 		btn_type = 0;