* [PATCHv3 1/7] cec: call enable_adap on s_log_addrs
2022-03-17 12:53 [PATCHv3 0/7] cec: various fixes and enhancements Hans Verkuil
@ 2022-03-17 12:53 ` Hans Verkuil
2022-03-17 12:53 ` [PATCHv3 2/7] cec: abort if the current transmit was canceled Hans Verkuil
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2022-03-17 12:53 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
Don't enable/disable the adapter if the first fh is opened or the
last fh is closed, instead do this when the adapter is configured
or unconfigured, and also when we enter Monitor All or Monitor Pin
mode for the first time or we exit the Monitor All/Pin mode for the
last time.
However, if needs_hpd is true, then do this when the physical
address is set or cleared: in that case the adapter typically is
powered by the HPD, so it really is disabled when the HPD is low.
This case (needs_hpd is true) was already handled in this way, so
this wasn't changed.
The problem with the old behavior was that if the HPD goes low when
no fh is open, and a transmit was in progress, then the adapter would
be disabled, typically stopping the transmit immediately which
leaves a partial message on the bus, which isn't nice and can confuse
some adapters.
It makes much more sense to disable it only when the adapter is
unconfigured and we're not monitoring the bus, since then you really
won't be using it anymore.
To keep track of this store a CEC activation count and call adap_enable
only when it goes from 0 to 1 or back to 0.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/cec/core/cec-adap.c | 174 ++++++++++++++++++++++--------
drivers/media/cec/core/cec-api.c | 18 +---
include/media/cec.h | 2 +
3 files changed, 130 insertions(+), 64 deletions(-)
diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 2e12331c12a9..1a095308f3ab 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1552,6 +1552,7 @@ static void cec_claim_log_addrs(struct cec_adapter *adap, bool block)
"ceccfg-%s", adap->name);
if (IS_ERR(adap->kthread_config)) {
adap->kthread_config = NULL;
+ adap->is_configuring = false;
} else if (block) {
mutex_unlock(&adap->lock);
wait_for_completion(&adap->config_completion);
@@ -1559,59 +1560,90 @@ static void cec_claim_log_addrs(struct cec_adapter *adap, bool block)
}
}
+/*
+ * Helper functions to enable/disable the CEC adapter.
+ *
+ * These functions are called with adap->lock held.
+ */
+static int cec_activate_cnt_inc(struct cec_adapter *adap)
+{
+ int ret;
+
+ if (adap->activate_cnt++)
+ return 0;
+
+ /* serialize adap_enable */
+ mutex_lock(&adap->devnode.lock);
+ adap->last_initiator = 0xff;
+ adap->transmit_in_progress = false;
+ ret = adap->ops->adap_enable(adap, true);
+ if (ret)
+ adap->activate_cnt--;
+ 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(adap->ops->adap_enable(adap, false));
+ adap->last_initiator = 0xff;
+ adap->transmit_in_progress = false;
+ 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.
*/
void __cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
{
+ bool becomes_invalid = phys_addr == CEC_PHYS_ADDR_INVALID;
+ bool is_invalid = adap->phys_addr == CEC_PHYS_ADDR_INVALID;
+
if (phys_addr == adap->phys_addr)
return;
- if (phys_addr != CEC_PHYS_ADDR_INVALID && adap->devnode.unregistered)
+ if (!becomes_invalid && adap->devnode.unregistered)
return;
dprintk(1, "new physical address %x.%x.%x.%x\n",
cec_phys_addr_exp(phys_addr));
- if (phys_addr == CEC_PHYS_ADDR_INVALID ||
- adap->phys_addr != CEC_PHYS_ADDR_INVALID) {
+ if (becomes_invalid || !is_invalid) {
adap->phys_addr = CEC_PHYS_ADDR_INVALID;
cec_post_state_event(adap);
cec_adap_unconfigure(adap);
- /* Disabling monitor all mode should always succeed */
- if (adap->monitor_all_cnt)
- WARN_ON(call_op(adap, adap_monitor_all_enable, false));
- /* serialize adap_enable */
- mutex_lock(&adap->devnode.lock);
- if (adap->needs_hpd || list_empty(&adap->devnode.fhs)) {
- WARN_ON(adap->ops->adap_enable(adap, false));
- adap->transmit_in_progress = false;
+ 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);
}
- mutex_unlock(&adap->devnode.lock);
- if (phys_addr == CEC_PHYS_ADDR_INVALID)
+ if (becomes_invalid)
return;
}
- /* serialize adap_enable */
- mutex_lock(&adap->devnode.lock);
- adap->last_initiator = 0xff;
- adap->transmit_in_progress = false;
-
- if (adap->needs_hpd || list_empty(&adap->devnode.fhs)) {
- if (adap->ops->adap_enable(adap, true)) {
- mutex_unlock(&adap->devnode.lock);
+ if (is_invalid && adap->needs_hpd) {
+ if (cec_activate_cnt_inc(adap))
return;
- }
- }
-
- if (adap->monitor_all_cnt &&
- call_op(adap, adap_monitor_all_enable, true)) {
- if (adap->needs_hpd || list_empty(&adap->devnode.fhs))
- WARN_ON(adap->ops->adap_enable(adap, false));
- mutex_unlock(&adap->devnode.lock);
- 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));
}
- mutex_unlock(&adap->devnode.lock);
adap->phys_addr = phys_addr;
cec_post_state_event(adap);
@@ -1676,6 +1708,8 @@ 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)
+ return 0;
cec_adap_unconfigure(adap);
adap->log_addrs.num_log_addrs = 0;
for (i = 0; i < CEC_MAX_LOG_ADDRS; i++)
@@ -1683,6 +1717,8 @@ 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);
return 0;
}
@@ -1816,6 +1852,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)
@@ -2119,20 +2161,37 @@ static int cec_receive_notify(struct cec_adapter *adap, struct cec_msg *msg,
*/
int cec_monitor_all_cnt_inc(struct cec_adapter *adap)
{
- int ret = 0;
+ int ret;
- if (adap->monitor_all_cnt == 0)
- ret = call_op(adap, adap_monitor_all_enable, 1);
- if (ret == 0)
- adap->monitor_all_cnt++;
+ 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) {
+ adap->monitor_all_cnt--;
+ if (!adap->needs_hpd)
+ cec_activate_cnt_dec(adap);
+ }
return ret;
}
void cec_monitor_all_cnt_dec(struct cec_adapter *adap)
{
- adap->monitor_all_cnt--;
- if (adap->monitor_all_cnt == 0)
- WARN_ON(call_op(adap, adap_monitor_all_enable, 0));
+ if (WARN_ON(!adap->monitor_all_cnt))
+ return;
+ 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);
}
/*
@@ -2142,20 +2201,37 @@ void cec_monitor_all_cnt_dec(struct cec_adapter *adap)
*/
int cec_monitor_pin_cnt_inc(struct cec_adapter *adap)
{
- int ret = 0;
+ int ret;
- if (adap->monitor_pin_cnt == 0)
- ret = call_op(adap, adap_monitor_pin_enable, 1);
- if (ret == 0)
- adap->monitor_pin_cnt++;
+ 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) {
+ adap->monitor_pin_cnt--;
+ if (!adap->needs_hpd)
+ cec_activate_cnt_dec(adap);
+ }
return ret;
}
void cec_monitor_pin_cnt_dec(struct cec_adapter *adap)
{
- adap->monitor_pin_cnt--;
- if (adap->monitor_pin_cnt == 0)
- WARN_ON(call_op(adap, adap_monitor_pin_enable, 0));
+ if (WARN_ON(!adap->monitor_pin_cnt))
+ return;
+ 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);
}
#ifdef CONFIG_DEBUG_FS
@@ -2169,6 +2245,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, "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",
@@ -2183,6 +2260,9 @@ int cec_adap_status(struct seq_file *file, void *priv)
if (adap->monitor_all_cnt)
seq_printf(file, "file handles in Monitor All mode: %u\n",
adap->monitor_all_cnt);
+ if (adap->monitor_pin_cnt)
+ seq_printf(file, "file handles in Monitor Pin mode: %u\n",
+ adap->monitor_pin_cnt);
if (adap->tx_timeouts) {
seq_printf(file, "transmit timeouts: %u\n",
adap->tx_timeouts);
diff --git a/drivers/media/cec/core/cec-api.c b/drivers/media/cec/core/cec-api.c
index d72ad48c9898..0284db12842b 100644
--- a/drivers/media/cec/core/cec-api.c
+++ b/drivers/media/cec/core/cec-api.c
@@ -586,18 +586,6 @@ static int cec_open(struct inode *inode, struct file *filp)
return err;
}
- /* serialize adap_enable */
- mutex_lock(&devnode->lock);
- if (list_empty(&devnode->fhs) &&
- !adap->needs_hpd &&
- adap->phys_addr == CEC_PHYS_ADDR_INVALID) {
- err = adap->ops->adap_enable(adap, true);
- if (err) {
- mutex_unlock(&devnode->lock);
- kfree(fh);
- return err;
- }
- }
filp->private_data = fh;
/* Queue up initial state events */
@@ -625,6 +613,7 @@ static int cec_open(struct inode *inode, struct file *filp)
}
#endif
+ mutex_lock(&devnode->lock);
mutex_lock(&devnode->lock_fhs);
list_add(&fh->list, &devnode->fhs);
mutex_unlock(&devnode->lock_fhs);
@@ -656,15 +645,10 @@ static int cec_release(struct inode *inode, struct file *filp)
cec_monitor_all_cnt_dec(adap);
mutex_unlock(&adap->lock);
- /* serialize adap_enable */
mutex_lock(&devnode->lock);
mutex_lock(&devnode->lock_fhs);
list_del(&fh->list);
mutex_unlock(&devnode->lock_fhs);
- if (cec_is_registered(adap) && list_empty(&devnode->fhs) &&
- !adap->needs_hpd && adap->phys_addr == CEC_PHYS_ADDR_INVALID) {
- WARN_ON(adap->ops->adap_enable(adap, false));
- }
mutex_unlock(&devnode->lock);
/* Unhook pending transmits from this filehandle. */
diff --git a/include/media/cec.h b/include/media/cec.h
index 77346f757036..97c5f5bfcbd0 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -185,6 +185,7 @@ 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
@@ -236,6 +237,7 @@ struct cec_adapter {
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCHv3 2/7] cec: abort if the current transmit was canceled
2022-03-17 12:53 [PATCHv3 0/7] cec: various fixes and enhancements Hans Verkuil
2022-03-17 12:53 ` [PATCHv3 1/7] cec: call enable_adap on s_log_addrs Hans Verkuil
@ 2022-03-17 12:53 ` Hans Verkuil
2022-03-17 12:53 ` [PATCHv3 3/7] cec: correctly pass on reply results Hans Verkuil
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2022-03-17 12:53 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
If a transmit-in-progress was canceled, then, once the transmit
is done, mark it as aborted and refrain from retrying the transmit.
To signal this situation the new transmit_in_progress_aborted field is
set to true.
The old implementation would just set adap->transmitting to NULL and
set adap->transmit_in_progress to false, but on the hardware level
the transmit was still ongoing. However, the framework would think
the transmit was aborted, and if a new transmit was issued, then
it could overwrite the HW buffer containing the old transmit with the
new transmit, leading to garbled data on the CEC bus.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/cec/core/cec-adap.c | 11 ++++++++---
include/media/cec.h | 6 ++++++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 1a095308f3ab..96968d18d7ac 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -421,7 +421,7 @@ static void cec_flush(struct cec_adapter *adap)
cec_data_cancel(data, CEC_TX_STATUS_ABORTED);
}
if (adap->transmitting)
- cec_data_cancel(adap->transmitting, CEC_TX_STATUS_ABORTED);
+ adap->transmit_in_progress_aborted = true;
/* Cancel the pending timeout work. */
list_for_each_entry_safe(data, n, &adap->wait_queue, list) {
@@ -572,6 +572,7 @@ int cec_thread_func(void *_adap)
if (data->attempts == 0)
data->attempts = attempts;
+ adap->transmit_in_progress_aborted = false;
/* Tell the adapter to transmit, cancel on error */
if (adap->ops->adap_transmit(adap, data->attempts,
signal_free_time, &data->msg))
@@ -599,6 +600,8 @@ void cec_transmit_done_ts(struct cec_adapter *adap, u8 status,
struct cec_msg *msg;
unsigned int attempts_made = arb_lost_cnt + nack_cnt +
low_drive_cnt + error_cnt;
+ bool done = status & (CEC_TX_STATUS_MAX_RETRIES | CEC_TX_STATUS_OK);
+ bool aborted = adap->transmit_in_progress_aborted;
dprintk(2, "%s: status 0x%02x\n", __func__, status);
if (attempts_made < 1)
@@ -619,6 +622,7 @@ void cec_transmit_done_ts(struct cec_adapter *adap, u8 status,
goto wake_thread;
}
adap->transmit_in_progress = false;
+ adap->transmit_in_progress_aborted = false;
msg = &data->msg;
@@ -639,8 +643,7 @@ void cec_transmit_done_ts(struct cec_adapter *adap, u8 status,
* the hardware didn't signal that it retried itself (by setting
* CEC_TX_STATUS_MAX_RETRIES), then we will retry ourselves.
*/
- if (data->attempts > attempts_made &&
- !(status & (CEC_TX_STATUS_MAX_RETRIES | CEC_TX_STATUS_OK))) {
+ if (!aborted && data->attempts > attempts_made && !done) {
/* Retry this message */
data->attempts -= attempts_made;
if (msg->timeout)
@@ -655,6 +658,8 @@ void cec_transmit_done_ts(struct cec_adapter *adap, u8 status,
goto wake_thread;
}
+ if (aborted && !done)
+ status |= CEC_TX_STATUS_ABORTED;
data->attempts = 0;
/* Always set CEC_TX_STATUS_MAX_RETRIES on error */
diff --git a/include/media/cec.h b/include/media/cec.h
index 97c5f5bfcbd0..31d704f36707 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -163,6 +163,11 @@ struct cec_adap_ops {
* @wait_queue: queue of transmits waiting for a reply
* @transmitting: CEC messages currently being transmitted
* @transmit_in_progress: true if a transmit is in progress
+ * @transmit_in_progress_aborted: true if a transmit is in progress is to be
+ * aborted. This happens if the logical address is
+ * invalidated while the transmit is ongoing. In that
+ * case the transmit will finish, but will not retransmit
+ * and be marked as ABORTED.
* @kthread_config: kthread used to configure a CEC adapter
* @config_completion: used to signal completion of the config kthread
* @kthread: main CEC processing thread
@@ -218,6 +223,7 @@ struct cec_adapter {
struct list_head wait_queue;
struct cec_data *transmitting;
bool transmit_in_progress;
+ bool transmit_in_progress_aborted;
struct task_struct *kthread_config;
struct completion config_completion;
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCHv3 3/7] cec: correctly pass on reply results
2022-03-17 12:53 [PATCHv3 0/7] cec: various fixes and enhancements Hans Verkuil
2022-03-17 12:53 ` [PATCHv3 1/7] cec: call enable_adap on s_log_addrs Hans Verkuil
2022-03-17 12:53 ` [PATCHv3 2/7] cec: abort if the current transmit was canceled Hans Verkuil
@ 2022-03-17 12:53 ` Hans Verkuil
2022-03-17 12:53 ` [PATCHv3 4/7] cec.h: add cec_msg_recv_is_rx/tx_result helpers Hans Verkuil
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2022-03-17 12:53 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
The results of non-blocking transmits were not correctly communicated
to userspace.
Specifically:
1) if a non-blocking transmit was canceled, then rx_status wasn't set to 0
as it should.
2) if the non-blocking transmit succeeded, but the corresponding reply
never arrived (aborted or timed out), then tx_status wasn't set to 0
as it should, and rx_status was hardcoded to ABORTED instead of the
actual reason, such as TIMEOUT. In addition, adap->ops->received() was
never called, so drivers that want to do message processing themselves
would not be informed of the failed reply.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/cec/core/cec-adap.c | 46 +++++++++++++++++++------------
1 file changed, 29 insertions(+), 17 deletions(-)
diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 96968d18d7ac..8c4aa208f8a0 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -366,38 +366,48 @@ static void cec_data_completed(struct cec_data *data)
/*
* A pending CEC transmit needs to be cancelled, either because the CEC
* adapter is disabled or the transmit takes an impossibly long time to
- * finish.
+ * finish, or the reply timed out.
*
* This function is called with adap->lock held.
*/
-static void cec_data_cancel(struct cec_data *data, u8 tx_status)
+static void cec_data_cancel(struct cec_data *data, u8 tx_status, u8 rx_status)
{
+ struct cec_adapter *adap = data->adap;
+
/*
* It's either the current transmit, or it is a pending
* transmit. Take the appropriate action to clear it.
*/
- if (data->adap->transmitting == data) {
- data->adap->transmitting = NULL;
+ if (adap->transmitting == data) {
+ adap->transmitting = NULL;
} else {
list_del_init(&data->list);
if (!(data->msg.tx_status & CEC_TX_STATUS_OK))
- if (!WARN_ON(!data->adap->transmit_queue_sz))
- data->adap->transmit_queue_sz--;
+ if (!WARN_ON(!adap->transmit_queue_sz))
+ adap->transmit_queue_sz--;
}
if (data->msg.tx_status & CEC_TX_STATUS_OK) {
data->msg.rx_ts = ktime_get_ns();
- data->msg.rx_status = CEC_RX_STATUS_ABORTED;
+ data->msg.rx_status = rx_status;
+ if (!data->blocking)
+ data->msg.tx_status = 0;
} else {
data->msg.tx_ts = ktime_get_ns();
data->msg.tx_status |= tx_status |
CEC_TX_STATUS_MAX_RETRIES;
data->msg.tx_error_cnt++;
data->attempts = 0;
+ if (!data->blocking)
+ data->msg.rx_status = 0;
}
/* Queue transmitted message for monitoring purposes */
- cec_queue_msg_monitor(data->adap, &data->msg, 1);
+ cec_queue_msg_monitor(adap, &data->msg, 1);
+
+ if (!data->blocking && data->msg.sequence && adap->ops->received)
+ /* Allow drivers to process the message first */
+ adap->ops->received(adap, &data->msg);
cec_data_completed(data);
}
@@ -418,7 +428,7 @@ static void cec_flush(struct cec_adapter *adap)
while (!list_empty(&adap->transmit_queue)) {
data = list_first_entry(&adap->transmit_queue,
struct cec_data, list);
- cec_data_cancel(data, CEC_TX_STATUS_ABORTED);
+ cec_data_cancel(data, CEC_TX_STATUS_ABORTED, 0);
}
if (adap->transmitting)
adap->transmit_in_progress_aborted = true;
@@ -426,7 +436,7 @@ static void cec_flush(struct cec_adapter *adap)
/* Cancel the pending timeout work. */
list_for_each_entry_safe(data, n, &adap->wait_queue, list) {
if (cancel_delayed_work(&data->work))
- cec_data_cancel(data, CEC_TX_STATUS_OK);
+ cec_data_cancel(data, CEC_TX_STATUS_OK, CEC_RX_STATUS_ABORTED);
/*
* If cancel_delayed_work returned false, then
* the cec_wait_timeout function is running,
@@ -516,7 +526,7 @@ int cec_thread_func(void *_adap)
adap->transmitting->msg.msg);
/* Just give up on this. */
cec_data_cancel(adap->transmitting,
- CEC_TX_STATUS_TIMEOUT);
+ CEC_TX_STATUS_TIMEOUT, 0);
} else {
pr_warn("cec-%s: transmit timed out\n", adap->name);
}
@@ -576,7 +586,7 @@ int cec_thread_func(void *_adap)
/* Tell the adapter to transmit, cancel on error */
if (adap->ops->adap_transmit(adap, data->attempts,
signal_free_time, &data->msg))
- cec_data_cancel(data, CEC_TX_STATUS_ABORTED);
+ cec_data_cancel(data, CEC_TX_STATUS_ABORTED, 0);
else
adap->transmit_in_progress = true;
@@ -738,9 +748,7 @@ static void cec_wait_timeout(struct work_struct *work)
/* Mark the message as timed out */
list_del_init(&data->list);
- data->msg.rx_ts = ktime_get_ns();
- data->msg.rx_status = CEC_RX_STATUS_TIMEOUT;
- cec_data_completed(data);
+ cec_data_cancel(data, CEC_TX_STATUS_OK, CEC_RX_STATUS_TIMEOUT);
unlock:
mutex_unlock(&adap->lock);
}
@@ -926,8 +934,12 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
mutex_lock(&adap->lock);
/* Cancel the transmit if it was interrupted */
- if (!data->completed)
- cec_data_cancel(data, CEC_TX_STATUS_ABORTED);
+ if (!data->completed) {
+ if (data->msg.tx_status & CEC_TX_STATUS_OK)
+ cec_data_cancel(data, CEC_TX_STATUS_OK, CEC_RX_STATUS_ABORTED);
+ else
+ cec_data_cancel(data, CEC_TX_STATUS_ABORTED, 0);
+ }
/* The transmit completed (possibly with an error) */
*msg = data->msg;
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCHv3 4/7] cec.h: add cec_msg_recv_is_rx/tx_result helpers
2022-03-17 12:53 [PATCHv3 0/7] cec: various fixes and enhancements Hans Verkuil
` (2 preceding siblings ...)
2022-03-17 12:53 ` [PATCHv3 3/7] cec: correctly pass on reply results Hans Verkuil
@ 2022-03-17 12:53 ` Hans Verkuil
2022-03-17 12:53 ` [PATCHv3 5/7] cec: use call_op and check for !unregistered Hans Verkuil
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2022-03-17 12:53 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
These two helper functions return true if the received message
contains the result of a previous non-blocking transmit. Either
the tx_status result (cec_msg_recv_is_tx_result) of the transmit,
or the rx_status result (cec_msg_recv_is_rx_result) of the reply
to the original transmit.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
include/uapi/linux/cec.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
index de936f5e446d..1d48da926216 100644
--- a/include/uapi/linux/cec.h
+++ b/include/uapi/linux/cec.h
@@ -142,6 +142,26 @@ static inline void cec_msg_set_reply_to(struct cec_msg *msg,
msg->reply = msg->timeout = 0;
}
+/**
+ * cec_msg_recv_is_tx_result - return true if this message contains the
+ * result of an earlier non-blocking transmit
+ * @msg: the message structure from CEC_RECEIVE
+ */
+static inline int cec_msg_recv_is_tx_result(const struct cec_msg *msg)
+{
+ return msg->sequence && msg->tx_status && !msg->rx_status;
+}
+
+/**
+ * cec_msg_recv_is_rx_result - return true if this message contains the
+ * reply of an earlier non-blocking transmit
+ * @msg: the message structure from CEC_RECEIVE
+ */
+static inline int cec_msg_recv_is_rx_result(const struct cec_msg *msg)
+{
+ return msg->sequence && !msg->tx_status && msg->rx_status;
+}
+
/* cec_msg flags field */
#define CEC_MSG_FL_REPLY_TO_FOLLOWERS (1 << 0)
#define CEC_MSG_FL_RAW (1 << 1)
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCHv3 5/7] cec: use call_op and check for !unregistered
2022-03-17 12:53 [PATCHv3 0/7] cec: various fixes and enhancements Hans Verkuil
` (3 preceding siblings ...)
2022-03-17 12:53 ` [PATCHv3 4/7] cec.h: add cec_msg_recv_is_rx/tx_result helpers Hans Verkuil
@ 2022-03-17 12:53 ` Hans Verkuil
2022-03-17 12:53 ` [PATCHv3 6/7] cec: add xfer_timeout_ms field Hans Verkuil
2022-03-17 12:53 ` [PATCHv3 7/7] cec: add optional adap_configured callback Hans Verkuil
6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2022-03-17 12:53 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
Use call_(void_)op consistently in the CEC core framework. Ditto
for the cec pin ops. And check if !adap->devnode.unregistered before
calling each op. This avoids calls to ops when the device has been
unregistered and the underlying hardware may be gone.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/cec/core/cec-adap.c | 37 ++++++++++-----------------
drivers/media/cec/core/cec-api.c | 6 +++--
drivers/media/cec/core/cec-core.c | 4 +--
drivers/media/cec/core/cec-pin-priv.h | 11 ++++++++
drivers/media/cec/core/cec-pin.c | 23 ++++++++---------
drivers/media/cec/core/cec-priv.h | 10 ++++++++
6 files changed, 51 insertions(+), 40 deletions(-)
diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 8c4aa208f8a0..d4fefd67ffe8 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -39,15 +39,6 @@ static void cec_fill_msg_report_features(struct cec_adapter *adap,
*/
#define CEC_XFER_TIMEOUT_MS (5 * 400 + 100)
-#define call_op(adap, op, arg...) \
- (adap->ops->op ? adap->ops->op(adap, ## arg) : 0)
-
-#define call_void_op(adap, op, arg...) \
- do { \
- if (adap->ops->op) \
- adap->ops->op(adap, ## arg); \
- } while (0)
-
static int cec_log_addr2idx(const struct cec_adapter *adap, u8 log_addr)
{
int i;
@@ -405,9 +396,9 @@ static void cec_data_cancel(struct cec_data *data, u8 tx_status, u8 rx_status)
/* Queue transmitted message for monitoring purposes */
cec_queue_msg_monitor(adap, &data->msg, 1);
- if (!data->blocking && data->msg.sequence && adap->ops->received)
+ if (!data->blocking && data->msg.sequence)
/* Allow drivers to process the message first */
- adap->ops->received(adap, &data->msg);
+ call_op(adap, received, &data->msg);
cec_data_completed(data);
}
@@ -584,8 +575,8 @@ int cec_thread_func(void *_adap)
adap->transmit_in_progress_aborted = false;
/* Tell the adapter to transmit, cancel on error */
- if (adap->ops->adap_transmit(adap, data->attempts,
- signal_free_time, &data->msg))
+ if (call_op(adap, adap_transmit, data->attempts,
+ signal_free_time, &data->msg))
cec_data_cancel(data, CEC_TX_STATUS_ABORTED, 0);
else
adap->transmit_in_progress = true;
@@ -1331,7 +1322,7 @@ static int cec_config_log_addr(struct cec_adapter *adap,
* Message not acknowledged, so this logical
* address is free to use.
*/
- err = adap->ops->adap_log_addr(adap, log_addr);
+ err = call_op(adap, adap_log_addr, log_addr);
if (err)
return err;
@@ -1348,9 +1339,8 @@ static int cec_config_log_addr(struct cec_adapter *adap,
*/
static void cec_adap_unconfigure(struct cec_adapter *adap)
{
- if (!adap->needs_hpd ||
- adap->phys_addr != CEC_PHYS_ADDR_INVALID)
- WARN_ON(adap->ops->adap_log_addr(adap, CEC_LOG_ADDR_INVALID));
+ 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;
@@ -1593,7 +1583,7 @@ static int cec_activate_cnt_inc(struct cec_adapter *adap)
mutex_lock(&adap->devnode.lock);
adap->last_initiator = 0xff;
adap->transmit_in_progress = false;
- ret = adap->ops->adap_enable(adap, true);
+ ret = call_op(adap, adap_enable, true);
if (ret)
adap->activate_cnt--;
mutex_unlock(&adap->devnode.lock);
@@ -1610,7 +1600,7 @@ static void cec_activate_cnt_dec(struct cec_adapter *adap)
/* serialize adap_enable */
mutex_lock(&adap->devnode.lock);
- WARN_ON(adap->ops->adap_enable(adap, false));
+ WARN_ON(call_op(adap, adap_enable, false));
adap->last_initiator = 0xff;
adap->transmit_in_progress = false;
mutex_unlock(&adap->devnode.lock);
@@ -1981,11 +1971,10 @@ static int cec_receive_notify(struct cec_adapter *adap, struct cec_msg *msg,
msg->msg[1] != CEC_MSG_CDC_MESSAGE)
return 0;
- if (adap->ops->received) {
- /* Allow drivers to process the message first */
- if (adap->ops->received(adap, msg) != -ENOMSG)
- return 0;
- }
+ /* Allow drivers to process the message first */
+ if (adap->ops->received && !adap->devnode.unregistered &&
+ adap->ops->received(adap, msg) != -ENOMSG)
+ return 0;
/*
* REPORT_PHYSICAL_ADDR, CEC_MSG_USER_CONTROL_PRESSED and
diff --git a/drivers/media/cec/core/cec-api.c b/drivers/media/cec/core/cec-api.c
index 0284db12842b..67dc79ef1705 100644
--- a/drivers/media/cec/core/cec-api.c
+++ b/drivers/media/cec/core/cec-api.c
@@ -595,7 +595,8 @@ static int cec_open(struct inode *inode, struct file *filp)
adap->conn_info.type != CEC_CONNECTOR_TYPE_NO_CONNECTOR;
cec_queue_event_fh(fh, &ev, 0);
#ifdef CONFIG_CEC_PIN
- if (adap->pin && adap->pin->ops->read_hpd) {
+ if (adap->pin && adap->pin->ops->read_hpd &&
+ !adap->devnode.unregistered) {
err = adap->pin->ops->read_hpd(adap);
if (err >= 0) {
ev.event = err ? CEC_EVENT_PIN_HPD_HIGH :
@@ -603,7 +604,8 @@ static int cec_open(struct inode *inode, struct file *filp)
cec_queue_event_fh(fh, &ev, 0);
}
}
- if (adap->pin && adap->pin->ops->read_5v) {
+ if (adap->pin && adap->pin->ops->read_5v &&
+ !adap->devnode.unregistered) {
err = adap->pin->ops->read_5v(adap);
if (err >= 0) {
ev.event = err ? CEC_EVENT_PIN_5V_HIGH :
diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index a3ab6a43fb14..6038be40b448 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -204,7 +204,7 @@ static ssize_t cec_error_inj_write(struct file *file,
line = strsep(&p, "\n");
if (!*line || *line == '#')
continue;
- if (!adap->ops->error_inj_parse_line(adap, line)) {
+ if (!call_op(adap, error_inj_parse_line, line)) {
kfree(buf);
return -EINVAL;
}
@@ -217,7 +217,7 @@ static int cec_error_inj_show(struct seq_file *sf, void *unused)
{
struct cec_adapter *adap = sf->private;
- return adap->ops->error_inj_show(adap, sf);
+ return call_op(adap, error_inj_show, sf);
}
static int cec_error_inj_open(struct inode *inode, struct file *file)
diff --git a/drivers/media/cec/core/cec-pin-priv.h b/drivers/media/cec/core/cec-pin-priv.h
index 7bad5a0b7cb7..8eb5819e6ccb 100644
--- a/drivers/media/cec/core/cec-pin-priv.h
+++ b/drivers/media/cec/core/cec-pin-priv.h
@@ -12,6 +12,17 @@
#include <linux/atomic.h>
#include <media/cec-pin.h>
+#define call_pin_op(pin, op, arg...) \
+ ((pin && pin->ops->op && !pin->adap->devnode.unregistered) ? \
+ pin->ops->op(pin->adap, ## arg) : 0)
+
+#define call_void_pin_op(pin, op, arg...) \
+ do { \
+ if (pin && pin->ops->op && \
+ !pin->adap->devnode.unregistered) \
+ pin->ops->op(pin->adap, ## arg); \
+ } while (0)
+
enum cec_pin_state {
/* CEC is off */
CEC_ST_OFF,
diff --git a/drivers/media/cec/core/cec-pin.c b/drivers/media/cec/core/cec-pin.c
index 21f0f749713e..78e4aef623bf 100644
--- a/drivers/media/cec/core/cec-pin.c
+++ b/drivers/media/cec/core/cec-pin.c
@@ -135,7 +135,7 @@ static void cec_pin_update(struct cec_pin *pin, bool v, bool force)
static bool cec_pin_read(struct cec_pin *pin)
{
- bool v = pin->ops->read(pin->adap);
+ bool v = call_pin_op(pin, read);
cec_pin_update(pin, v, false);
return v;
@@ -143,13 +143,13 @@ static bool cec_pin_read(struct cec_pin *pin)
static void cec_pin_low(struct cec_pin *pin)
{
- pin->ops->low(pin->adap);
+ call_void_pin_op(pin, low);
cec_pin_update(pin, false, false);
}
static bool cec_pin_high(struct cec_pin *pin)
{
- pin->ops->high(pin->adap);
+ call_void_pin_op(pin, high);
return cec_pin_read(pin);
}
@@ -1086,7 +1086,7 @@ static int cec_pin_thread_func(void *_adap)
CEC_PIN_IRQ_UNCHANGED)) {
case CEC_PIN_IRQ_DISABLE:
if (irq_enabled) {
- pin->ops->disable_irq(adap);
+ call_void_pin_op(pin, disable_irq);
irq_enabled = false;
}
cec_pin_high(pin);
@@ -1097,7 +1097,7 @@ static int cec_pin_thread_func(void *_adap)
case CEC_PIN_IRQ_ENABLE:
if (irq_enabled)
break;
- pin->enable_irq_failed = !pin->ops->enable_irq(adap);
+ pin->enable_irq_failed = !call_pin_op(pin, enable_irq);
if (pin->enable_irq_failed) {
cec_pin_to_idle(pin);
hrtimer_start(&pin->timer, ns_to_ktime(0),
@@ -1112,8 +1112,8 @@ static int cec_pin_thread_func(void *_adap)
if (kthread_should_stop())
break;
}
- if (pin->ops->disable_irq && irq_enabled)
- pin->ops->disable_irq(adap);
+ if (irq_enabled)
+ call_void_pin_op(pin, disable_irq);
hrtimer_cancel(&pin->timer);
cec_pin_read(pin);
cec_pin_to_idle(pin);
@@ -1207,7 +1207,7 @@ static void cec_pin_adap_status(struct cec_adapter *adap,
seq_printf(file, "state: %s\n", states[pin->state].name);
seq_printf(file, "tx_bit: %d\n", pin->tx_bit);
seq_printf(file, "rx_bit: %d\n", pin->rx_bit);
- seq_printf(file, "cec pin: %d\n", pin->ops->read(adap));
+ seq_printf(file, "cec pin: %d\n", call_pin_op(pin, read));
seq_printf(file, "cec pin events dropped: %u\n",
pin->work_pin_events_dropped_cnt);
seq_printf(file, "irq failed: %d\n", pin->enable_irq_failed);
@@ -1260,8 +1260,7 @@ static void cec_pin_adap_status(struct cec_adapter *adap,
pin->rx_data_bit_too_long_cnt = 0;
pin->rx_low_drive_cnt = 0;
pin->tx_low_drive_cnt = 0;
- if (pin->ops->status)
- pin->ops->status(adap, file);
+ call_void_pin_op(pin, status, file);
}
static int cec_pin_adap_monitor_all_enable(struct cec_adapter *adap,
@@ -1277,7 +1276,7 @@ static void cec_pin_adap_free(struct cec_adapter *adap)
{
struct cec_pin *pin = adap->pin;
- if (pin->ops->free)
+ if (pin && pin->ops->free)
pin->ops->free(adap);
adap->pin = NULL;
kfree(pin);
@@ -1287,7 +1286,7 @@ static int cec_pin_received(struct cec_adapter *adap, struct cec_msg *msg)
{
struct cec_pin *pin = adap->pin;
- if (pin->ops->received)
+ if (pin->ops->received && !adap->devnode.unregistered)
return pin->ops->received(adap, msg);
return -ENOMSG;
}
diff --git a/drivers/media/cec/core/cec-priv.h b/drivers/media/cec/core/cec-priv.h
index 9bbd05053d42..b78df931aa74 100644
--- a/drivers/media/cec/core/cec-priv.h
+++ b/drivers/media/cec/core/cec-priv.h
@@ -17,6 +17,16 @@
pr_info("cec-%s: " fmt, adap->name, ## arg); \
} while (0)
+#define call_op(adap, op, arg...) \
+ ((adap->ops->op && !adap->devnode.unregistered) ? \
+ adap->ops->op(adap, ## arg) : 0)
+
+#define call_void_op(adap, op, arg...) \
+ do { \
+ if (adap->ops->op && !adap->devnode.unregistered) \
+ adap->ops->op(adap, ## arg); \
+ } while (0)
+
/* devnode to cec_adapter */
#define to_cec_adapter(node) container_of(node, struct cec_adapter, devnode)
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCHv3 6/7] cec: add xfer_timeout_ms field
2022-03-17 12:53 [PATCHv3 0/7] cec: various fixes and enhancements Hans Verkuil
` (4 preceding siblings ...)
2022-03-17 12:53 ` [PATCHv3 5/7] cec: use call_op and check for !unregistered Hans Verkuil
@ 2022-03-17 12:53 ` Hans Verkuil
2022-03-17 12:53 ` [PATCHv3 7/7] cec: add optional adap_configured callback Hans Verkuil
6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2022-03-17 12:53 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
Allow drivers to change the transmit timeout value, i.e. after how
long should a transmit be considered 'lost', i.e. the corresponding
cec_transmit_done_ts was never called.
Some CEC devices have their own timeout, and so this timeout value must be
longer than that hardware timeout value. If it is shorter then the
framework would consider the transmit lost, even though it is effectively
still in progress at the hardware level.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/cec/core/cec-adap.c | 17 +++--------------
drivers/media/cec/core/cec-core.c | 14 ++++++++++++++
include/media/cec.h | 3 +++
3 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index d4fefd67ffe8..d359c9972d06 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -27,18 +27,6 @@ static void cec_fill_msg_report_features(struct cec_adapter *adap,
struct cec_msg *msg,
unsigned int la_idx);
-/*
- * 400 ms is the time it takes for one 16 byte message to be
- * transferred and 5 is the maximum number of retries. Add
- * another 100 ms as a margin. So if the transmit doesn't
- * finish before that time something is really wrong and we
- * have to time out.
- *
- * This is a sign that something it really wrong and a warning
- * will be issued.
- */
-#define CEC_XFER_TIMEOUT_MS (5 * 400 + 100)
-
static int cec_log_addr2idx(const struct cec_adapter *adap, u8 log_addr)
{
int i;
@@ -483,7 +471,7 @@ int cec_thread_func(void *_adap)
kthread_should_stop() ||
(!adap->transmit_in_progress &&
!list_empty(&adap->transmit_queue)),
- msecs_to_jiffies(CEC_XFER_TIMEOUT_MS));
+ msecs_to_jiffies(adap->xfer_timeout_ms));
timeout = err == 0;
} else {
/* Otherwise we just wait for something to happen. */
@@ -509,7 +497,8 @@ int cec_thread_func(void *_adap)
* adapter driver, or the CEC bus is in some weird
* state. On rare occasions it can happen if there is
* so much traffic on the bus that the adapter was
- * unable to transmit for CEC_XFER_TIMEOUT_MS (2.1s).
+ * unable to transmit for xfer_timeout_ms (2.1s by
+ * default).
*/
if (adap->transmitting) {
pr_warn("cec-%s: message %*ph timed out\n", adap->name,
diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index 6038be40b448..af358e901b5f 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -20,6 +20,18 @@
#define CEC_NUM_DEVICES 256
#define CEC_NAME "cec"
+/*
+ * 400 ms is the time it takes for one 16 byte message to be
+ * transferred and 5 is the maximum number of retries. Add
+ * another 100 ms as a margin. So if the transmit doesn't
+ * finish before that time something is really wrong and we
+ * have to time out.
+ *
+ * This is a sign that something it really wrong and a warning
+ * will be issued.
+ */
+#define CEC_XFER_TIMEOUT_MS (5 * 400 + 100)
+
int cec_debug;
module_param_named(debug, cec_debug, int, 0644);
MODULE_PARM_DESC(debug, "debug level (0-2)");
@@ -331,6 +343,8 @@ int cec_register_adapter(struct cec_adapter *adap,
adap->owner = parent->driver->owner;
adap->devnode.dev.parent = parent;
+ if (!adap->xfer_timeout_ms)
+ adap->xfer_timeout_ms = CEC_XFER_TIMEOUT_MS;
#ifdef CONFIG_MEDIA_CEC_RC
if (adap->capabilities & CEC_CAP_RC) {
diff --git a/include/media/cec.h b/include/media/cec.h
index 31d704f36707..80340c9eb0f2 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -168,6 +168,8 @@ struct cec_adap_ops {
* invalidated while the transmit is ongoing. In that
* case the transmit will finish, but will not retransmit
* and be marked as ABORTED.
+ * @xfer_timeout_ms: the transfer timeout in ms.
+ * If 0, then timeout after 2.1 ms.
* @kthread_config: kthread used to configure a CEC adapter
* @config_completion: used to signal completion of the config kthread
* @kthread: main CEC processing thread
@@ -224,6 +226,7 @@ struct cec_adapter {
struct cec_data *transmitting;
bool transmit_in_progress;
bool transmit_in_progress_aborted;
+ unsigned int xfer_timeout_ms;
struct task_struct *kthread_config;
struct completion config_completion;
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCHv3 7/7] cec: add optional adap_configured callback
2022-03-17 12:53 [PATCHv3 0/7] cec: various fixes and enhancements Hans Verkuil
` (5 preceding siblings ...)
2022-03-17 12:53 ` [PATCHv3 6/7] cec: add xfer_timeout_ms field Hans Verkuil
@ 2022-03-17 12:53 ` Hans Verkuil
6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2022-03-17 12:53 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
This new optional callback is called when the adapter is fully configured
or fully unconfigured. Some drivers may have to take action when this
happens.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
Documentation/driver-api/media/cec-core.rst | 13 ++++++++++++-
drivers/media/cec/core/cec-adap.c | 2 ++
include/media/cec.h | 1 +
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/driver-api/media/cec-core.rst b/Documentation/driver-api/media/cec-core.rst
index c6194ee81c41..ae0d20798edc 100644
--- a/Documentation/driver-api/media/cec-core.rst
+++ b/Documentation/driver-api/media/cec-core.rst
@@ -109,6 +109,7 @@ your driver:
int (*adap_monitor_all_enable)(struct cec_adapter *adap, bool enable);
int (*adap_monitor_pin_enable)(struct cec_adapter *adap, bool enable);
int (*adap_log_addr)(struct cec_adapter *adap, u8 logical_addr);
+ void (*adap_configured)(struct cec_adapter *adap, bool configured);
int (*adap_transmit)(struct cec_adapter *adap, u8 attempts,
u32 signal_free_time, struct cec_msg *msg);
void (*adap_status)(struct cec_adapter *adap, struct seq_file *file);
@@ -117,7 +118,7 @@ your driver:
/* Error injection callbacks */
...
- /* High-level callbacks */
+ /* High-level callback */
...
};
@@ -178,6 +179,16 @@ can receive directed messages to that address.
Note that adap_log_addr must return 0 if logical_addr is CEC_LOG_ADDR_INVALID.
+Called when the adapter is fully configured or unconfigured::
+
+ void (*adap_configured)(struct cec_adapter *adap, bool configured);
+
+If configured == true, then the adapter is fully configured, i.e. all logical
+addresses have been successfully claimed. If configured == false, then the
+adapter is unconfigured. If the driver has to take specific actions after
+(un)configuration, then that can be done through this optional callback.
+
+
To transmit a new message::
int (*adap_transmit)(struct cec_adapter *adap, u8 attempts,
diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index d359c9972d06..d7d9057000b5 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1336,6 +1336,7 @@ static void cec_adap_unconfigure(struct cec_adapter *adap)
cec_flush(adap);
wake_up_interruptible(&adap->kthread_waitq);
cec_post_state_event(adap);
+ call_void_op(adap, adap_configured, false);
}
/*
@@ -1517,6 +1518,7 @@ static int cec_config_thread_func(void *arg)
adap->kthread_config = NULL;
complete(&adap->config_completion);
mutex_unlock(&adap->lock);
+ call_void_op(adap, adap_configured, true);
return 0;
unconfigure:
diff --git a/include/media/cec.h b/include/media/cec.h
index 80340c9eb0f2..6f13b0222aa3 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -118,6 +118,7 @@ struct cec_adap_ops {
int (*adap_monitor_all_enable)(struct cec_adapter *adap, bool enable);
int (*adap_monitor_pin_enable)(struct cec_adapter *adap, bool enable);
int (*adap_log_addr)(struct cec_adapter *adap, u8 logical_addr);
+ void (*adap_configured)(struct cec_adapter *adap, bool configured);
int (*adap_transmit)(struct cec_adapter *adap, u8 attempts,
u32 signal_free_time, struct cec_msg *msg);
void (*adap_status)(struct cec_adapter *adap, struct seq_file *file);
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread