Hi Hans,
On 04/10/18 12:08, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> The TX FIFO has to be cleared if the transmit failed due to e.g.
> a NACK condition, otherwise the hardware will keep trying to
> transmit the message.
>
> An attempt was made to do this, but it was done after the call to
> cec_transmit_done, which can cause a race condition since the call
> to cec_transmit_done can cause a new transmit to be issued, and
> then attempting to clear the TX FIFO will actually clear the new
> transmit instead of the old transmit and the new transmit simply
> never happens.
>
> By clearing the FIFO before transmit_done is called this race
> is fixed.
>
> Note that there is no reason to clear the FIFO if the transmit
> was successful, so the attempt to clear the FIFO in that case
> was dropped.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
> drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c | 35 ++++++++++++-------------
> 1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> index 340383150fb9..dee66a5101b5 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> @@ -106,6 +106,22 @@ static void hdmi_cec_received_msg(struct hdmi_core_data *core)
> }
> }
>
> +static bool hdmi_cec_clear_tx_fifo(struct cec_adapter *adap)
> +{
> + struct hdmi_core_data *core = cec_get_drvdata(adap);
> + int retry = HDMI_CORE_CEC_RETRY;
> + int temp;
> +
> + REG_FLD_MOD(core->base, HDMI_CEC_DBG_3, 0x1, 7, 7);
> + while (retry) {
> + temp = hdmi_read_reg(core->base, HDMI_CEC_DBG_3);
> + if (FLD_GET(temp, 7, 7) == 0)
> + break;
This is fine, but as you're using the helper macros already, there's
REG_GET:
REG_GET(core->base, HDMI_CEC_DBG_3, 7, 7)
which removes the need for temp. Are you sure this works reliably?
Usually when polling a register bit, I like to measure real-world-time
in some way to ensure I actually poll for a certain amount of time.
And just a matter of opinion, but I would've written:
while (retry) {
if (!REG_GET(..))
return true;
retry--;
}
return false;
> + retry--;
> + }
> + return retry != 0;
> +}
> +
> void hdmi4_cec_irq(struct hdmi_core_data *core)
> {
> u32 stat0 = hdmi_read_reg(core->base, HDMI_CEC_INT_STATUS_0);
> @@ -117,36 +133,19 @@ void hdmi4_cec_irq(struct hdmi_core_data *core)
> if (stat0 & 0x20) {
> cec_transmit_done(core->adap, CEC_TX_STATUS_OK,
> 0, 0, 0, 0);
> - REG_FLD_MOD(core->base, HDMI_CEC_DBG_3, 0x1, 7, 7);
> } else if (stat1 & 0x02) {
> u32 dbg3 = hdmi_read_reg(core->base, HDMI_CEC_DBG_3);
>
> + hdmi_cec_clear_tx_fifo(core->adap);
Would a dev_err be ok here?
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
