Re: [PATCHv9 14/15] cec: s5p-cec: Add s5p-cec driver

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:

• 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?