On Tue, Jan 22, 2019 at 12:44:07PM +0000, Igor Russkikh wrote:
>
> >> -#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);
Hi Igor
err = readx_poll_timeout(hw_atl_itr_res_irq_get, self, alt_itr_res,
alt_itr_res == 0, 10, 1000);
The advantage of using readx_poll_timeout is that it is used by lots
of other drivers and works. It is much better to use core
infrastructure, then build your own.
Andrew
