netdev - Re: [PATCH net 5/5] net: aquantia: added err var into AQ_HW_WAIT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190122133424.GB3634@lunn.ch>
Date:   Tue, 22 Jan 2019 14:34:24 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Igor Russkikh <Igor.Russkikh@...antia.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Nikita Danilov <Nikita.Danilov@...antia.com>
Subject: Re: [PATCH net 5/5] net: aquantia: added err var into AQ_HW_WAIT_FOR
 construct

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