On 10/12/2015 02:33 PM, Kamil Debski wrote:
> Hi,
>
> On 12 October 2015 at 12:50, Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>> On 10/06/2015 12:32 AM, Russell King - ARM Linux wrote:
>>> On Mon, Sep 07, 2015 at 03:44:43PM +0200, Hans Verkuil wrote:
>>>> + if (status & CEC_STATUS_TX_DONE) {
>>>> + if (status & CEC_STATUS_TX_ERROR) {
>>>> + dev_dbg(cec->dev, "CEC_STATUS_TX_ERROR set\n");
>>>> + cec->tx = STATE_ERROR;
>>>> + } else {
>>>> + dev_dbg(cec->dev, "CEC_STATUS_TX_DONE\n");
>>>> + cec->tx = STATE_DONE;
>>>> + }
>>>> + s5p_clr_pending_tx(cec);
>>>> + }
>>>
>>> Your CEC implementation seems to be written around the idea that there
>>> are only two possible outcomes from a CEC message - "done" and "error",
>>> which get translated to:
>>
>> This code is for the Samsung exynos CEC implementation. Marek, is this all
>> that the exynos CEC hardware returns?
>
> The possible status values that are implemented in the CEC framework
> are following:
>
> +/* cec status field */
> +#define CEC_TX_STATUS_OK (0)
> +#define CEC_TX_STATUS_ARB_LOST (1 << 0)
> +#define CEC_TX_STATUS_RETRY_TIMEOUT (1 << 1)
> +#define CEC_TX_STATUS_FEATURE_ABORT (1 << 2)
> +#define CEC_TX_STATUS_REPLY_TIMEOUT (1 << 3)
> +#define CEC_RX_STATUS_READY (0)
>
> The only two ones I could match with the Exynos CEC module status bits are
> CEC_TX_STATUS_OK and CEC_TX_STATUS_RETRY_TIMEOUT.
>
> The status bits in Exynos HW are:
> - Tx_Error
> - Tx_Done
> - Tx_Transferring
> - Tx_Running
> - Tx_Bytes_Transferred
>
> - Tx_Wait
> - Tx_Sending_Status_Bit
> - Tx_Sending_Hdr_Blk
> - Tx_Sending_Data_Blk
> - Tx_Latest_Initiator
>
> - Tx_Wait_SFT_Succ
> - Tx_Wait_SFT_New
> - Tx_Wait_SFT_Retran
> - Tx_Retrans_Cnt
> - Tx_ACK_Failed
So are these all intermediate states? And every transfer always ends with Tx_Done or
Tx_Error state?
It does look that way...
Regards,
Hans
>
>>
>>>
>>>> + case STATE_DONE:
>>>> + cec_transmit_done(cec->adap, CEC_TX_STATUS_OK);
>>>> + cec->tx = STATE_IDLE;
>>>> + break;
>>>> + case STATE_ERROR:
>>>> + cec_transmit_done(cec->adap, CEC_TX_STATUS_RETRY_TIMEOUT);
>>>> + cec->tx = STATE_IDLE;
>>>
>>> "okay" and "retry_timeout". So, if we have an adapter which can report
>>> (eg) a NACK, we have to report it as the obscure "retry timeout" status?
>>> Why this obscure naming - why can't we have something that uses the
>>> terminology in the spec?
>>>
>>
>> Actually, a NACK should lead to a re-transmission (up to 5 times), see CEC 7.1.
>> The assumption of the CEC framework is that this is done by the CEC adapter
>> driver, not by the framework. So if after repeated retransmissions there is
>> still no Ack, the CEC_TX_STATUS_RETRY_TIMEOUT error is returned. I could
>> change this to _MAX_RETRIES_REACHED if you prefer.
>>
>> The CEC_TX_STATUS_ macros were based on what the adv drivers support (except
>> for CEC_TX_STATUS_REPLY_TIMEOUT which is specific to the framework).
>>
>> Regards,
>>
>> Hans
>
> Best wishes,
> Kamil
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
