Commit f9d0ecbf authored by Hans Verkuil's avatar Hans Verkuil Committed by Mauro Carvalho Chehab

media: cec: correctly pass on reply results

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: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@kernel.org>
parent 590a8e56
...@@ -366,38 +366,48 @@ static void cec_data_completed(struct cec_data *data) ...@@ -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 * A pending CEC transmit needs to be cancelled, either because the CEC
* adapter is disabled or the transmit takes an impossibly long time to * 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. * 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 * It's either the current transmit, or it is a pending
* transmit. Take the appropriate action to clear it. * transmit. Take the appropriate action to clear it.
*/ */
if (data->adap->transmitting == data) { if (adap->transmitting == data) {
data->adap->transmitting = NULL; adap->transmitting = NULL;
} else { } else {
list_del_init(&data->list); list_del_init(&data->list);
if (!(data->msg.tx_status & CEC_TX_STATUS_OK)) if (!(data->msg.tx_status & CEC_TX_STATUS_OK))
if (!WARN_ON(!data->adap->transmit_queue_sz)) if (!WARN_ON(!adap->transmit_queue_sz))
data->adap->transmit_queue_sz--; adap->transmit_queue_sz--;
} }
if (data->msg.tx_status & CEC_TX_STATUS_OK) { if (data->msg.tx_status & CEC_TX_STATUS_OK) {
data->msg.rx_ts = ktime_get_ns(); 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 { } else {
data->msg.tx_ts = ktime_get_ns(); data->msg.tx_ts = ktime_get_ns();
data->msg.tx_status |= tx_status | data->msg.tx_status |= tx_status |
CEC_TX_STATUS_MAX_RETRIES; CEC_TX_STATUS_MAX_RETRIES;
data->msg.tx_error_cnt++; data->msg.tx_error_cnt++;
data->attempts = 0; data->attempts = 0;
if (!data->blocking)
data->msg.rx_status = 0;
} }
/* Queue transmitted message for monitoring purposes */ /* 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); cec_data_completed(data);
} }
...@@ -418,7 +428,7 @@ static void cec_flush(struct cec_adapter *adap) ...@@ -418,7 +428,7 @@ static void cec_flush(struct cec_adapter *adap)
while (!list_empty(&adap->transmit_queue)) { while (!list_empty(&adap->transmit_queue)) {
data = list_first_entry(&adap->transmit_queue, data = list_first_entry(&adap->transmit_queue,
struct cec_data, list); struct cec_data, list);
cec_data_cancel(data, CEC_TX_STATUS_ABORTED); cec_data_cancel(data, CEC_TX_STATUS_ABORTED, 0);
} }
if (adap->transmitting) if (adap->transmitting)
adap->transmit_in_progress_aborted = true; adap->transmit_in_progress_aborted = true;
...@@ -426,7 +436,7 @@ static void cec_flush(struct cec_adapter *adap) ...@@ -426,7 +436,7 @@ static void cec_flush(struct cec_adapter *adap)
/* Cancel the pending timeout work. */ /* Cancel the pending timeout work. */
list_for_each_entry_safe(data, n, &adap->wait_queue, list) { list_for_each_entry_safe(data, n, &adap->wait_queue, list) {
if (cancel_delayed_work(&data->work)) 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 * If cancel_delayed_work returned false, then
* the cec_wait_timeout function is running, * the cec_wait_timeout function is running,
...@@ -516,7 +526,7 @@ int cec_thread_func(void *_adap) ...@@ -516,7 +526,7 @@ int cec_thread_func(void *_adap)
adap->transmitting->msg.msg); adap->transmitting->msg.msg);
/* Just give up on this. */ /* Just give up on this. */
cec_data_cancel(adap->transmitting, cec_data_cancel(adap->transmitting,
CEC_TX_STATUS_TIMEOUT); CEC_TX_STATUS_TIMEOUT, 0);
} else { } else {
pr_warn("cec-%s: transmit timed out\n", adap->name); pr_warn("cec-%s: transmit timed out\n", adap->name);
} }
...@@ -576,7 +586,7 @@ int cec_thread_func(void *_adap) ...@@ -576,7 +586,7 @@ int cec_thread_func(void *_adap)
/* Tell the adapter to transmit, cancel on error */ /* Tell the adapter to transmit, cancel on error */
if (adap->ops->adap_transmit(adap, data->attempts, if (adap->ops->adap_transmit(adap, data->attempts,
signal_free_time, &data->msg)) signal_free_time, &data->msg))
cec_data_cancel(data, CEC_TX_STATUS_ABORTED); cec_data_cancel(data, CEC_TX_STATUS_ABORTED, 0);
else else
adap->transmit_in_progress = true; adap->transmit_in_progress = true;
...@@ -738,9 +748,7 @@ static void cec_wait_timeout(struct work_struct *work) ...@@ -738,9 +748,7 @@ static void cec_wait_timeout(struct work_struct *work)
/* Mark the message as timed out */ /* Mark the message as timed out */
list_del_init(&data->list); list_del_init(&data->list);
data->msg.rx_ts = ktime_get_ns(); cec_data_cancel(data, CEC_TX_STATUS_OK, CEC_RX_STATUS_TIMEOUT);
data->msg.rx_status = CEC_RX_STATUS_TIMEOUT;
cec_data_completed(data);
unlock: unlock:
mutex_unlock(&adap->lock); mutex_unlock(&adap->lock);
} }
...@@ -926,8 +934,12 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg, ...@@ -926,8 +934,12 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
mutex_lock(&adap->lock); mutex_lock(&adap->lock);
/* Cancel the transmit if it was interrupted */ /* Cancel the transmit if it was interrupted */
if (!data->completed) if (!data->completed) {
cec_data_cancel(data, CEC_TX_STATUS_ABORTED); 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) */ /* The transmit completed (possibly with an error) */
*msg = data->msg; *msg = data->msg;
...@@ -1603,7 +1615,7 @@ static void cec_activate_cnt_dec(struct cec_adapter *adap) ...@@ -1603,7 +1615,7 @@ static void cec_activate_cnt_dec(struct cec_adapter *adap)
adap->transmit_in_progress = false; adap->transmit_in_progress = false;
adap->transmit_in_progress_aborted = false; adap->transmit_in_progress_aborted = false;
if (adap->transmitting) if (adap->transmitting)
cec_data_cancel(adap->transmitting, CEC_TX_STATUS_ABORTED); cec_data_cancel(adap->transmitting, CEC_TX_STATUS_ABORTED, 0);
mutex_unlock(&adap->devnode.lock); mutex_unlock(&adap->devnode.lock);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment