Re: [PATCH net 5/5] net: aquantia: added err var into AQ_HW_WAIT_FOR construct

[Andrew Lunn <andrew@lunn.ch>]

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