[PATCH 0/7] CEC framework fixes

[PATCH 0/7] CEC framework fixes

[Hans Verkuil]

When testing what happens when randomly configuring and unconfiguring
a CEC adapter, and simultaneously setting an invalidating the physical
address, and starting and stopping monitoring and pin monitoring using
two cec-gpio instances, a whole bunch of race conditions were found.

This patch series addresses them.

Hans Verkuil (7):
  cec-pin: disabling the adapter cannot call kthread_stop
  cec-pin: don't zero work_pin_num_events in adap_enable
  cec-adap.c: don't unconfigure if already unconfigured
  cec-adap.c: stop trying LAs on CEC_TX_STATUS_TIMEOUT
  cec-adap.c: fix is_configuring state
  cec-adap.c: reconfigure if the PA changes during configuration
  cec-adap: drop activate_cnt, use state info instead

 drivers/media/cec/core/cec-adap.c | 188 ++++++++++++++----------------
 drivers/media/cec/core/cec-pin.c  |  57 +++++----
 include/media/cec.h               |   6 +-
 3 files changed, 124 insertions(+), 127 deletions(-)

-- 
2.34.1

[PATCH 1/7] cec-pin: disabling the adapter cannot call kthread_stop

[Hans Verkuil]

When the adap_enable callback is called the adap->lock is held.
When disabling the adapter it attempts to stop the kthread that
deals with receiving and transmitting messages. However, kthread_stop
waits for the thread to stop, so all that time adap->lock is held.

Unfortunately, the kernel thread itself can call functions that take
that same lock, so a deadlock can occur.

Change the logic to keep the kernel thread running and instead when
disabling the adapter, just set the pin to high, go to idle and then
to state OFF and disable the interrupt. Only stop the kernel thread
when the adapter is deleted.

This way disabling the adapter will not wait for anything and the
deadlock is avoided.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/core/cec-pin.c | 54 +++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/drivers/media/cec/core/cec-pin.c b/drivers/media/cec/core/cec-pin.c
index 78e4aef623bf..4bd7be4e2edf 100644
--- a/drivers/media/cec/core/cec-pin.c
+++ b/drivers/media/cec/core/cec-pin.c
@@ -1037,11 +1037,14 @@ static int cec_pin_thread_func(void *_adap)
 
 	for (;;) {
 		wait_event_interruptible(pin->kthread_waitq,
-			kthread_should_stop() ||
-			pin->work_rx_msg.len ||
-			pin->work_tx_status ||
-			atomic_read(&pin->work_irq_change) ||
-			atomic_read(&pin->work_pin_num_events));
+					 kthread_should_stop() ||
+					 pin->work_rx_msg.len ||
+					 pin->work_tx_status ||
+					 atomic_read(&pin->work_irq_change) ||
+					 atomic_read(&pin->work_pin_num_events));
+
+		if (kthread_should_stop())
+			break;
 
 		if (pin->work_rx_msg.len) {
 			struct cec_msg *msg = &pin->work_rx_msg;
@@ -1090,6 +1093,8 @@ static int cec_pin_thread_func(void *_adap)
 				irq_enabled = false;
 			}
 			cec_pin_high(pin);
+			if (pin->state == CEC_ST_OFF)
+				break;
 			cec_pin_to_idle(pin);
 			hrtimer_start(&pin->timer, ns_to_ktime(0),
 				      HRTIMER_MODE_REL);
@@ -1109,15 +1114,7 @@ static int cec_pin_thread_func(void *_adap)
 		default:
 			break;
 		}
-		if (kthread_should_stop())
-			break;
 	}
-	if (irq_enabled)
-		call_void_pin_op(pin, disable_irq);
-	hrtimer_cancel(&pin->timer);
-	cec_pin_read(pin);
-	cec_pin_to_idle(pin);
-	pin->state = CEC_ST_OFF;
 	return 0;
 }
 
@@ -1134,16 +1131,28 @@ static int cec_pin_adap_enable(struct cec_adapter *adap, bool enable)
 		pin->tx_msg.len = 0;
 		pin->timer_ts = ns_to_ktime(0);
 		atomic_set(&pin->work_irq_change, CEC_PIN_IRQ_UNCHANGED);
-		pin->kthread = kthread_run(cec_pin_thread_func, adap,
-					   "cec-pin");
-		if (IS_ERR(pin->kthread)) {
-			pr_err("cec-pin: kernel_thread() failed\n");
-			return PTR_ERR(pin->kthread);
+		if (!pin->kthread) {
+			pin->kthread = kthread_run(cec_pin_thread_func, adap,
+						   "cec-pin");
+			if (IS_ERR(pin->kthread)) {
+				int err = PTR_ERR(pin->kthread);
+
+				pr_err("cec-pin: kernel_thread() failed\n");
+				pin->kthread = NULL;
+				return err;
+			}
 		}
 		hrtimer_start(&pin->timer, ns_to_ktime(0),
 			      HRTIMER_MODE_REL);
-	} else {
-		kthread_stop(pin->kthread);
+	} else if (pin->kthread) {
+		hrtimer_cancel(&pin->timer);
+		cec_pin_high(pin);
+		cec_pin_to_idle(pin);
+		pin->state = CEC_ST_OFF;
+		pin->work_tx_status = 0;
+		atomic_set(&pin->work_pin_num_events, 0);
+		atomic_set(&pin->work_irq_change, CEC_PIN_IRQ_DISABLE);
+		wake_up_interruptible(&pin->kthread_waitq);
 	}
 	return 0;
 }
@@ -1276,7 +1285,10 @@ static void cec_pin_adap_free(struct cec_adapter *adap)
 {
 	struct cec_pin *pin = adap->pin;
 
-	if (pin && pin->ops->free)
+	if (pin->kthread)
+		kthread_stop(pin->kthread);
+	pin->kthread = NULL;
+	if (pin->ops->free)
 		pin->ops->free(adap);
 	adap->pin = NULL;
 	kfree(pin);
-- 
2.34.1

[PATCH 2/7] cec-pin: don't zero work_pin_num_events in adap_enable

[Hans Verkuil]

It's OK to keep the pending pin events when disabling or
enabling the 'adapter'. Zeroing this can cause a race condition
if this happens when the pin kthread is handling a pin event
and calls atomic_dec later, causing work_pin_num_events to become
negative.

Just leave pending events in the queue, they'll be read eventually.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/core/cec-pin.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/media/cec/core/cec-pin.c b/drivers/media/cec/core/cec-pin.c
index 4bd7be4e2edf..68353c5dc501 100644
--- a/drivers/media/cec/core/cec-pin.c
+++ b/drivers/media/cec/core/cec-pin.c
@@ -1123,9 +1123,6 @@ static int cec_pin_adap_enable(struct cec_adapter *adap, bool enable)
 	struct cec_pin *pin = adap->pin;
 
 	if (enable) {
-		atomic_set(&pin->work_pin_num_events, 0);
-		pin->work_pin_events_rd = pin->work_pin_events_wr = 0;
-		pin->work_pin_events_dropped = false;
 		cec_pin_read(pin);
 		cec_pin_to_idle(pin);
 		pin->tx_msg.len = 0;
@@ -1150,7 +1147,6 @@ static int cec_pin_adap_enable(struct cec_adapter *adap, bool enable)
 		cec_pin_to_idle(pin);
 		pin->state = CEC_ST_OFF;
 		pin->work_tx_status = 0;
-		atomic_set(&pin->work_pin_num_events, 0);
 		atomic_set(&pin->work_irq_change, CEC_PIN_IRQ_DISABLE);
 		wake_up_interruptible(&pin->kthread_waitq);
 	}
@@ -1338,6 +1334,7 @@ struct cec_adapter *cec_pin_allocate_adapter(const struct cec_pin_ops *pin_ops,
 		return ERR_PTR(-ENOMEM);
 	pin->ops = pin_ops;
 	hrtimer_init(&pin->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	atomic_set(&pin->work_pin_num_events, 0);
 	pin->timer.function = cec_pin_timer;
 	init_waitqueue_head(&pin->kthread_waitq);
 	pin->tx_custom_low_usecs = CEC_TIM_CUSTOM_DEFAULT;
-- 
2.34.1

[PATCH 3/7] cec-adap.c: don't unconfigure if already unconfigured

[Hans Verkuil]

The __cec_s_log_addrs() function can configure or unconfigure the
adapter. The ioctl handler in cec-api.c will prevent it from being
called to configure the adapter if it was already configured (or in
the process of configuring). But it can still be called to unconfigure
an already unconfigured adapter, and it didn't check for that.

This can cause cec_activate_cnt_dec() to be called too often, causing
a WARN_ON.

Instead first check if adap->log_addrs.num_log_addrs == 0 and return
since in that case the adapter is already unconfigured.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/core/cec-adap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index e789aec7455c..b47280fa3b87 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1709,9 +1709,10 @@ int __cec_s_log_addrs(struct cec_adapter *adap,
 		return -ENODEV;
 
 	if (!log_addrs || log_addrs->num_log_addrs == 0) {
-		if (!adap->is_configuring && !adap->is_configured)
+		if (!adap->log_addrs.num_log_addrs)
 			return 0;
-		cec_adap_unconfigure(adap);
+		if (adap->is_configuring || adap->is_configured)
+			cec_adap_unconfigure(adap);
 		adap->log_addrs.num_log_addrs = 0;
 		for (i = 0; i < CEC_MAX_LOG_ADDRS; i++)
 			adap->log_addrs.log_addr[i] = CEC_LOG_ADDR_INVALID;
-- 
2.34.1

[PATCH 4/7] cec-adap.c: stop trying LAs on CEC_TX_STATUS_TIMEOUT

[Hans Verkuil]

If, while trying to claim a free logical address, a POLL message
times out, then abort this process. A CEC_TX_STATUS_TIMEOUT
should be handled the same as a CEC_TX_STATUS_ABORTED.

This avoids a situation where transmits time out due to a
driver or hardware bug and it takes ages before the attempt
to find available free logical addresses finishes.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/core/cec-adap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index b47280fa3b87..004121e3582a 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1282,10 +1282,11 @@ static int cec_config_log_addr(struct cec_adapter *adap,
 			return err;
 
 		/*
-		 * The message was aborted due to a disconnect or
+		 * The message was aborted or timed out due to a disconnect or
 		 * unconfigure, just bail out.
 		 */
-		if (msg.tx_status & CEC_TX_STATUS_ABORTED)
+		if (msg.tx_status &
+		    (CEC_TX_STATUS_ABORTED | CEC_TX_STATUS_TIMEOUT))
 			return -EINTR;
 		if (msg.tx_status & CEC_TX_STATUS_OK)
 			return 0;
-- 
2.34.1

[PATCH 5/7] cec-adap.c: fix is_configuring state

[Hans Verkuil]

If an adapter is trying to claim a free logical address then it is
in the 'is_configuring' state. If during that process the cable is
disconnected (HPD goes low, which in turn invalidates the physical
address), then cec_adap_unconfigure() is called, and that set the
is_configuring boolean to false, even though the thread that's
trying to claim an LA is still running.

Don't touch the is_configuring bool in cec_adap_unconfigure(), it
will eventually be cleared by the thread. By making that change
the cec_config_log_addr() function also had to change: it was
aborting if is_configuring became false (since that is what
cec_adap_unconfigure() did), but that no longer works. Instead
check if the physical address is invalid. That is a much
more appropriate check anyway.

This fixes a bug where the the adapter could be disabled even
though the device was still configuring. This could cause POLL
transmits to time out.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/core/cec-adap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 004121e3582a..6c235503dd92 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1275,7 +1275,7 @@ static int cec_config_log_addr(struct cec_adapter *adap,
 		 * While trying to poll the physical address was reset
 		 * and the adapter was unconfigured, so bail out.
 		 */
-		if (!adap->is_configuring)
+		if (adap->phys_addr == CEC_PHYS_ADDR_INVALID)
 			return -EINTR;
 
 		if (err)
@@ -1332,7 +1332,6 @@ static void cec_adap_unconfigure(struct cec_adapter *adap)
 	if (!adap->needs_hpd || adap->phys_addr != CEC_PHYS_ADDR_INVALID)
 		WARN_ON(call_op(adap, adap_log_addr, CEC_LOG_ADDR_INVALID));
 	adap->log_addrs.log_addr_mask = 0;
-	adap->is_configuring = false;
 	adap->is_configured = false;
 	cec_flush(adap);
 	wake_up_interruptible(&adap->kthread_waitq);
@@ -1526,9 +1525,10 @@ static int cec_config_thread_func(void *arg)
 	for (i = 0; i < las->num_log_addrs; i++)
 		las->log_addr[i] = CEC_LOG_ADDR_INVALID;
 	cec_adap_unconfigure(adap);
+	adap->is_configuring = false;
 	adap->kthread_config = NULL;
-	mutex_unlock(&adap->lock);
 	complete(&adap->config_completion);
+	mutex_unlock(&adap->lock);
 	return 0;
 }
 
-- 
2.34.1

[PATCH 6/7] cec-adap.c: reconfigure if the PA changes during configuration

[Hans Verkuil]

If the physical address changes (i.e. becomes invalid, then valid again)
while the adapter is still claiming free logical addresses, then trigger
a reconfiguration since any claimed LAs may now be stale.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/core/cec-adap.c | 20 +++++++++++++++++++-
 include/media/cec.h               |  2 ++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 6c235503dd92..38c43a37133d 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1278,6 +1278,10 @@ static int cec_config_log_addr(struct cec_adapter *adap,
 		if (adap->phys_addr == CEC_PHYS_ADDR_INVALID)
 			return -EINTR;
 
+		/* Also bail out if the PA changed while configuring. */
+		if (adap->must_reconfigure)
+			return -EINTR;
+
 		if (err)
 			return err;
 
@@ -1405,6 +1409,7 @@ static int cec_config_thread_func(void *arg)
 	if (las->log_addr_type[0] == CEC_LOG_ADDR_TYPE_UNREGISTERED)
 		goto configured;
 
+reconfigure:
 	for (i = 0; i < las->num_log_addrs; i++) {
 		unsigned int type = las->log_addr_type[i];
 		const u8 *la_list;
@@ -1427,6 +1432,13 @@ static int cec_config_thread_func(void *arg)
 			last_la = la_list[0];
 
 		err = cec_config_log_addr(adap, i, last_la);
+
+		if (adap->must_reconfigure) {
+			adap->must_reconfigure = false;
+			las->log_addr_mask = 0;
+			goto reconfigure;
+		}
+
 		if (err > 0) /* Reused last LA */
 			continue;
 
@@ -1472,6 +1484,7 @@ static int cec_config_thread_func(void *arg)
 		las->log_addr[i] = CEC_LOG_ADDR_INVALID;
 	adap->is_configured = true;
 	adap->is_configuring = false;
+	adap->must_reconfigure = false;
 	cec_post_state_event(adap);
 
 	/*
@@ -1526,6 +1539,7 @@ static int cec_config_thread_func(void *arg)
 		las->log_addr[i] = CEC_LOG_ADDR_INVALID;
 	cec_adap_unconfigure(adap);
 	adap->is_configuring = false;
+	adap->must_reconfigure = false;
 	adap->kthread_config = NULL;
 	complete(&adap->config_completion);
 	mutex_unlock(&adap->lock);
@@ -1649,7 +1663,11 @@ void __cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
 
 	adap->phys_addr = phys_addr;
 	cec_post_state_event(adap);
-	if (adap->log_addrs.num_log_addrs)
+	if (!adap->log_addrs.num_log_addrs)
+		return;
+	if (adap->is_configuring)
+		adap->must_reconfigure = true;
+	else
 		cec_claim_log_addrs(adap, block);
 }
 
diff --git a/include/media/cec.h b/include/media/cec.h
index 6f13b0222aa3..6c9b41fe9802 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -184,6 +184,7 @@ struct cec_adap_ops {
  *	in order to transmit or receive CEC messages. This is usually a HW
  *	limitation.
  * @is_configuring:	the CEC adapter is configuring (i.e. claiming LAs)
+ * @must_reconfigure:	while configuring, the PA changed, so reclaim LAs
  * @is_configured:	the CEC adapter is configured (i.e. has claimed LAs)
  * @cec_pin_is_high:	if true then the CEC pin is high. Only used with the
  *	CEC pin framework.
@@ -243,6 +244,7 @@ struct cec_adapter {
 	u16 phys_addr;
 	bool needs_hpd;
 	bool is_configuring;
+	bool must_reconfigure;
 	bool is_configured;
 	bool cec_pin_is_high;
 	bool adap_controls_phys_addr;
-- 
2.34.1

[PATCH 7/7] cec-adap: drop activate_cnt, use state info instead

[Hans Verkuil]

Using an activation counter to decide when the enable or disable the
cec adapter is not the best approach and can lead to race conditions.

Change this to determining the current status of the adapter, and
enable or disable the adapter accordingly.

It now only needs to be called whenever there is a chance that the
state changes, and it can handle enabling/disabling monitoring as
well if needed.

This simplifies the code and it should be a more robust approach as well.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/core/cec-adap.c | 152 ++++++++++++------------------
 include/media/cec.h               |   4 +-
 2 files changed, 61 insertions(+), 95 deletions(-)

diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 38c43a37133d..8bf91b5a7d0e 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1574,47 +1574,59 @@ static void cec_claim_log_addrs(struct cec_adapter *adap, bool block)
 }
 
 /*
- * Helper functions to enable/disable the CEC adapter.
+ * Helper function to enable/disable the CEC adapter.
  *
- * These functions are called with adap->lock held.
+ * This function is called with adap->lock held.
  */
-static int cec_activate_cnt_inc(struct cec_adapter *adap)
+static int cec_adap_enable(struct cec_adapter *adap)
 {
-	int ret;
+	bool enable;
+	int ret = 0;
+
+	enable = adap->monitor_all_cnt || adap->monitor_pin_cnt ||
+		 adap->log_addrs.num_log_addrs;
+	if (adap->needs_hpd)
+		enable = enable && adap->phys_addr != CEC_PHYS_ADDR_INVALID;
 
-	if (adap->activate_cnt++)
+	if (enable == adap->is_enabled)
 		return 0;
 
 	/* serialize adap_enable */
 	mutex_lock(&adap->devnode.lock);
-	adap->last_initiator = 0xff;
-	adap->transmit_in_progress = false;
-	ret = call_op(adap, adap_enable, true);
-	if (ret)
-		adap->activate_cnt--;
+	if (enable) {
+		adap->last_initiator = 0xff;
+		adap->transmit_in_progress = false;
+		ret = adap->ops->adap_enable(adap, true);
+		if (!ret) {
+			/*
+			 * Enable monitor-all/pin modes if needed. We warn, but
+			 * continue if this fails as this is not a critical error.
+			 */
+			if (adap->monitor_all_cnt)
+				WARN_ON(call_op(adap, adap_monitor_all_enable, true));
+			if (adap->monitor_pin_cnt)
+				WARN_ON(call_op(adap, adap_monitor_pin_enable, true));
+		}
+	} else {
+		/* Disable monitor-all/pin modes if needed (needs_hpd == 1) */
+		if (adap->monitor_all_cnt)
+			WARN_ON(call_op(adap, adap_monitor_all_enable, false));
+		if (adap->monitor_pin_cnt)
+			WARN_ON(call_op(adap, adap_monitor_pin_enable, false));
+		WARN_ON(adap->ops->adap_enable(adap, false));
+		adap->last_initiator = 0xff;
+		adap->transmit_in_progress = false;
+		adap->transmit_in_progress_aborted = false;
+		if (adap->transmitting)
+			cec_data_cancel(adap->transmitting, CEC_TX_STATUS_ABORTED, 0);
+	}
+	if (!ret)
+		adap->is_enabled = enable;
+	wake_up_interruptible(&adap->kthread_waitq);
 	mutex_unlock(&adap->devnode.lock);
 	return ret;
 }
 
-static void cec_activate_cnt_dec(struct cec_adapter *adap)
-{
-	if (WARN_ON(!adap->activate_cnt))
-		return;
-
-	if (--adap->activate_cnt)
-		return;
-
-	/* serialize adap_enable */
-	mutex_lock(&adap->devnode.lock);
-	WARN_ON(call_op(adap, adap_enable, false));
-	adap->last_initiator = 0xff;
-	adap->transmit_in_progress = false;
-	adap->transmit_in_progress_aborted = false;
-	if (adap->transmitting)
-		cec_data_cancel(adap->transmitting, CEC_TX_STATUS_ABORTED, 0);
-	mutex_unlock(&adap->devnode.lock);
-}
-
 /* Set a new physical address and send an event notifying userspace of this.
  *
  * This function is called with adap->lock held.
@@ -1635,33 +1647,16 @@ void __cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
 		adap->phys_addr = CEC_PHYS_ADDR_INVALID;
 		cec_post_state_event(adap);
 		cec_adap_unconfigure(adap);
-		if (becomes_invalid && adap->needs_hpd) {
-			/* Disable monitor-all/pin modes if needed */
-			if (adap->monitor_all_cnt)
-				WARN_ON(call_op(adap, adap_monitor_all_enable, false));
-			if (adap->monitor_pin_cnt)
-				WARN_ON(call_op(adap, adap_monitor_pin_enable, false));
-			cec_activate_cnt_dec(adap);
-			wake_up_interruptible(&adap->kthread_waitq);
-		}
-		if (becomes_invalid)
+		if (becomes_invalid) {
+			cec_adap_enable(adap);
 			return;
-	}
-
-	if (is_invalid && adap->needs_hpd) {
-		if (cec_activate_cnt_inc(adap))
-			return;
-		/*
-		 * Re-enable monitor-all/pin modes if needed. We warn, but
-		 * continue if this fails as this is not a critical error.
-		 */
-		if (adap->monitor_all_cnt)
-			WARN_ON(call_op(adap, adap_monitor_all_enable, true));
-		if (adap->monitor_pin_cnt)
-			WARN_ON(call_op(adap, adap_monitor_pin_enable, true));
+		}
 	}
 
 	adap->phys_addr = phys_addr;
+	if (is_invalid)
+		cec_adap_enable(adap);
+
 	cec_post_state_event(adap);
 	if (!adap->log_addrs.num_log_addrs)
 		return;
@@ -1722,6 +1717,7 @@ int __cec_s_log_addrs(struct cec_adapter *adap,
 		      struct cec_log_addrs *log_addrs, bool block)
 {
 	u16 type_mask = 0;
+	int err;
 	int i;
 
 	if (adap->devnode.unregistered)
@@ -1738,8 +1734,7 @@ int __cec_s_log_addrs(struct cec_adapter *adap,
 		adap->log_addrs.osd_name[0] = '\0';
 		adap->log_addrs.vendor_id = CEC_VENDOR_ID_NONE;
 		adap->log_addrs.cec_version = CEC_OP_CEC_VERSION_2_0;
-		if (!adap->needs_hpd)
-			cec_activate_cnt_dec(adap);
+		cec_adap_enable(adap);
 		return 0;
 	}
 
@@ -1873,17 +1868,12 @@ int __cec_s_log_addrs(struct cec_adapter *adap,
 		       sizeof(log_addrs->features[i]));
 	}
 
-	if (!adap->needs_hpd && !adap->is_configuring && !adap->is_configured) {
-		int ret = cec_activate_cnt_inc(adap);
-
-		if (ret)
-			return ret;
-	}
 	log_addrs->log_addr_mask = adap->log_addrs.log_addr_mask;
 	adap->log_addrs = *log_addrs;
-	if (adap->phys_addr != CEC_PHYS_ADDR_INVALID)
+	err = cec_adap_enable(adap);
+	if (!err && adap->phys_addr != CEC_PHYS_ADDR_INVALID)
 		cec_claim_log_addrs(adap, block);
-	return 0;
+	return err;
 }
 
 int cec_s_log_addrs(struct cec_adapter *adap,
@@ -2186,20 +2176,9 @@ int cec_monitor_all_cnt_inc(struct cec_adapter *adap)
 	if (adap->monitor_all_cnt++)
 		return 0;
 
-	if (!adap->needs_hpd) {
-		ret = cec_activate_cnt_inc(adap);
-		if (ret) {
-			adap->monitor_all_cnt--;
-			return ret;
-		}
-	}
-
-	ret = call_op(adap, adap_monitor_all_enable, true);
-	if (ret) {
+	ret = cec_adap_enable(adap);
+	if (ret)
 		adap->monitor_all_cnt--;
-		if (!adap->needs_hpd)
-			cec_activate_cnt_dec(adap);
-	}
 	return ret;
 }
 
@@ -2210,8 +2189,7 @@ void cec_monitor_all_cnt_dec(struct cec_adapter *adap)
 	if (--adap->monitor_all_cnt)
 		return;
 	WARN_ON(call_op(adap, adap_monitor_all_enable, false));
-	if (!adap->needs_hpd)
-		cec_activate_cnt_dec(adap);
+	cec_adap_enable(adap);
 }
 
 /*
@@ -2226,20 +2204,9 @@ int cec_monitor_pin_cnt_inc(struct cec_adapter *adap)
 	if (adap->monitor_pin_cnt++)
 		return 0;
 
-	if (!adap->needs_hpd) {
-		ret = cec_activate_cnt_inc(adap);
-		if (ret) {
-			adap->monitor_pin_cnt--;
-			return ret;
-		}
-	}
-
-	ret = call_op(adap, adap_monitor_pin_enable, true);
-	if (ret) {
+	ret = cec_adap_enable(adap);
+	if (ret)
 		adap->monitor_pin_cnt--;
-		if (!adap->needs_hpd)
-			cec_activate_cnt_dec(adap);
-	}
 	return ret;
 }
 
@@ -2250,8 +2217,7 @@ void cec_monitor_pin_cnt_dec(struct cec_adapter *adap)
 	if (--adap->monitor_pin_cnt)
 		return;
 	WARN_ON(call_op(adap, adap_monitor_pin_enable, false));
-	if (!adap->needs_hpd)
-		cec_activate_cnt_dec(adap);
+	cec_adap_enable(adap);
 }
 
 #ifdef CONFIG_DEBUG_FS
@@ -2265,7 +2231,7 @@ int cec_adap_status(struct seq_file *file, void *priv)
 	struct cec_data *data;
 
 	mutex_lock(&adap->lock);
-	seq_printf(file, "activation count: %u\n", adap->activate_cnt);
+	seq_printf(file, "enabled: %d\n", adap->is_enabled);
 	seq_printf(file, "configured: %d\n", adap->is_configured);
 	seq_printf(file, "configuring: %d\n", adap->is_configuring);
 	seq_printf(file, "phys_addr: %x.%x.%x.%x\n",
diff --git a/include/media/cec.h b/include/media/cec.h
index 6c9b41fe9802..abee41ae02d0 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -183,6 +183,7 @@ struct cec_adap_ops {
  * @needs_hpd:		if true, then the HDMI HotPlug Detect pin must be high
  *	in order to transmit or receive CEC messages. This is usually a HW
  *	limitation.
+ * @is_enabled:		the CEC adapter is enabled
  * @is_configuring:	the CEC adapter is configuring (i.e. claiming LAs)
  * @must_reconfigure:	while configuring, the PA changed, so reclaim LAs
  * @is_configured:	the CEC adapter is configured (i.e. has claimed LAs)
@@ -194,7 +195,6 @@ struct cec_adap_ops {
  *	Drivers that need this can set this field to true after the
  *	cec_allocate_adapter() call.
  * @last_initiator:	the initiator of the last transmitted message.
- * @activate_cnt:	number of times that CEC is activated
  * @monitor_all_cnt:	number of filehandles monitoring all msgs
  * @monitor_pin_cnt:	number of filehandles monitoring pin changes
  * @follower_cnt:	number of filehandles in follower mode
@@ -243,13 +243,13 @@ struct cec_adapter {
 
 	u16 phys_addr;
 	bool needs_hpd;
+	bool is_enabled;
 	bool is_configuring;
 	bool must_reconfigure;
 	bool is_configured;
 	bool cec_pin_is_high;
 	bool adap_controls_phys_addr;
 	u8 last_initiator;
-	u32 activate_cnt;
 	u32 monitor_all_cnt;
 	u32 monitor_pin_cnt;
 	u32 follower_cnt;
-- 
2.34.1