>> -#define AQ_HW_WAIT_FOR(_B_, _US_, _N_) \
>> +#define AQ_HW_WAIT_FOR(_B_, _US_, _N_, _err_) \
>> do { \
>> unsigned int AQ_HW_WAIT_FOR_i; \
>> for (AQ_HW_WAIT_FOR_i = _N_; (!(_B_)) && (AQ_HW_WAIT_FOR_i);\
>> @@ -31,7 +31,7 @@ do { \
>> udelay(_US_); \
>> } \
>> if (!AQ_HW_WAIT_FOR_i) {\
>> - err = -ETIME; \
>> + *(_err_) = -ETIME; \
>> } \
>> } while (0)
>
> Hi Igor
>
> How about throwing this horrible macro away and using one of the
> readx_poll_timeout() variants.
Hi Andrew,
Thanks I was not aware of that macro.
The original macro actually differs a bit in a sense that it does not require
a 'value' local variable to be used. Also aq driver used a number of wrapper
functions to hide the actual readl and mmio access.
Thus its hard to adopt readx_poll_timeout() in its current form.
WAIT_FOR is normally used like
AQ_HW_WAIT_FOR(hw_atl_itr_res_irq_get(self) == 0, 1000U, 10U, &err);
So there is no direct access to register offset and value. Thats an expression
waiting for
logical condition to happen, not just register value to change.
I was under impression statement expressions (macro returning values) should
not be used because
of compatibility, but since its in use may be its the better to rewrite this as
#define AQ_HW_WAIT_FOR(_B_, _US_, _N_) \
({ \
unsigned int AQ_HW_WAIT_FOR_i; \
for (AQ_HW_WAIT_FOR_i = _N_; (!(_B_)) && (AQ_HW_WAIT_FOR_i);\
--AQ_HW_WAIT_FOR_i) {\
udelay(_US_); \
} \
AQ_HW_WAIT_FOR_i ? EOK : -ETIMEDOUT;\
})
So it could be used as
err = AQ_HW_WAIT_FOR(hw_atl_itr_res_irq_get(self) == 0, 1000U, 10U);
and may be worth renaming it to something with _poll_timeout suffix as well?
Regards,
Igor
