Skip to contentSkip to conversation

FAILED: patch "[PATCH] media: cec: check 'transmit_in_progress', not 'transmitting'" failed to apply to 5.5-stable tree

PATCH [email protected] 2 participants 2 messages Feb 9, 2020

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.5-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From d51224b73d18d207912f15ad4eb7a4b456682729 Mon Sep 17 00:00:00 2001
From: Hans Verkuil <[email protected]>
Date: Wed, 11 Dec 2019 12:47:57 +0100
Subject: [PATCH] media: cec: check 'transmit_in_progress', not 'transmitting'

Currently wait_event_interruptible_timeout is called in cec_thread_func() when adap->transmitting is set. But if the adapter is unconfigured while transmitting, then adap->transmitting is set to NULL. But the hardware is still actually transmitting the message, and that's indicated by adap->transmit_in_progress and we should wait until that is finished or times out before transmitting new messages.

As the original commit says: adap->transmitting is the userspace view, adap->transmit_in_progress reflects the hardware state.

However, if adap->transmitting is NULL and adap->transmit_in_progress is true, then wait_event_interruptible is called (no timeout), which can get stuck indefinitely if the CEC driver is flaky and never marks the transmit-in-progress as 'done'.

So test against transmit_in_progress when deciding whether to use the timeout variant or not, instead of testing against adap->transmitting.

Fixes 32804fcb612b ("media: cec: keep track of outstanding transmits")
Cc <[email protected]> # for v4.19 and up
drivers/media/cec/cec-adap.c +12 -8
index 1060e633b623..6c95dc471d4c 100644
@@ -465,7 +465,7 @@ int cec_thread_func(void *_adap)
 		bool timeout = false;
 		u8 attempts;
 
-		if (adap->transmitting) {
+		if (adap->transmit_in_progress) {
 			int err;
 
 			/*
@@ -500,7 +500,7 @@ int cec_thread_func(void *_adap)
 			goto unlock;
 		}
 
-		if (adap->transmitting && timeout) {
+		if (adap->transmit_in_progress && timeout) {
 			/*
 			 * If we timeout, then log that. Normally this does
 			 * not happen and it is an indication of a faulty CEC
@@ -509,14 +509,18 @@ int cec_thread_func(void *_adap)
 			 * so much traffic on the bus that the adapter was
 			 * unable to transmit for CEC_XFER_TIMEOUT_MS (2.1s).
 			 */
-			pr_warn("cec-%s: message %*ph timed out\n", adap->name,
-				adap->transmitting->msg.len,
-				adap->transmitting->msg.msg);
+			if (adap->transmitting) {
+				pr_warn("cec-%s: message %*ph timed out\n", adap->name,
+					adap->transmitting->msg.len,
+					adap->transmitting->msg.msg);
+				/* Just give up on this. */
+				cec_data_cancel(adap->transmitting,
+						CEC_TX_STATUS_TIMEOUT);
+			} else {
+				pr_warn("cec-%s: transmit timed out\n", adap->name);
+			}
 			adap->transmit_in_progress = false;
 			adap->tx_timeouts++;
-			/* Just give up on this. */
-			cec_data_cancel(adap->transmitting,
-					CEC_TX_STATUS_TIMEOUT);
 			goto unlock;
 		}
 
maintainer
#1 lore raw
Quoted 32 lines · On Sun, Feb 09, 2020 at 12:48:05PM +0100, [email protected] wrote:

The patch below does not apply to the 5.5-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From d51224b73d18d207912f15ad4eb7a4b456682729 Mon Sep 17 00:00:00 2001
From: Hans Verkuil <[email protected]>
Date: Wed, 11 Dec 2019 12:47:57 +0100
Subject: [PATCH] media: cec: check 'transmit_in_progress', not 'transmitting'

Currently wait_event_interruptible_timeout is called in cec_thread_func() when adap->transmitting is set. But if the adapter is unconfigured while transmitting, then adap->transmitting is set to NULL. But the hardware is still actually transmitting the message, and that's indicated by adap->transmit_in_progress and we should wait until that is finished or times out before transmitting new messages.

As the original commit says: adap->transmitting is the userspace view, adap->transmit_in_progress reflects the hardware state.

However, if adap->transmitting is NULL and adap->transmit_in_progress is true, then wait_event_interruptible is called (no timeout), which can get stuck indefinitely if the CEC driver is flaky and never marks the transmit-in-progress as 'done'.

Quoted 7 lines

So test against transmit_in_progress when deciding whether to use the timeout variant or not, instead of testing against adap->transmitting.

Fixes 32804fcb612b ("media: cec: keep track of outstanding transmits")
Cc <[email protected]> # for v4.19 and up

Duplicate commit:

ac479b51f3f4 ("media: cec: check 'transmit_in_progress', not 'transmitting'") d51224b73d18 ("media: cec: check 'transmit_in_progress', not 'transmitting'")

Signature · 2 lines
Thanks,
Sasha