On Wed, Jun 28, 2017 at 08:39:33PM +1000, Jonathan Liu wrote:
u32 int_status;u32 fifo_status;/* Read needs empty flag unset, write needs full flag unset */u32 flag = read ? SUN4I_HDMI_DDC_FIFO_STATUS_EMPTY :SUN4I_HDMI_DDC_FIFO_STATUS_FULL;int ret;/* Wait until error or FIFO ready */ret = readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_INT_STATUS_REG,int_status,is_err_status(int_status) ||is_fifo_flag_unset(hdmi, &fifo_status, flag),min(len, SUN4I_HDMI_DDC_FIFO_SIZE) * byte_time,100000);if (is_err_status(int_status))return -EIO;if (ret)return -ETIMEDOUT;Why not just have ret = readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_FIFO_STATUS_REG, reg, !(reg & flag), 100, 100000);
if (ret < 0) if (is_err_status()) return -EIO; return ret;
If I check error status after readl_poll_timeout and there is an error (e.g. the I2C address does not have a corresponding device connected or nothing connected to HDMI port) it will keep checking the fifo status even though error bit is set in the int status and then timeout after 100 ms. If it checks the int status register at the same time, it will error after 100 nanoseconds. I don't want to introduce unnecessary delays considering part of the reason for adding this driver to make it more usable for non-standard use cases.
Well, polling for 100ms doesn't seem great either. What was the rationale behind that timeout?
And we can also reverse the check and look at the INT_STATUS register. The errors will be there, and we can program the threshold we want in both directions and use the DDC_FIFO_Request_Interrupt_Status bit.
Maxime