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: <1bac2c2c-2c60-5043-3b74-0b20138a72c1@aquantia.com>
Date:   Wed, 23 Jan 2019 09:49:25 +0000
From:   Igor Russkikh <Igor.Russkikh@...antia.com>
To:     Andrew Lunn <andrew@...n.ch>
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


> 
> 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.

Hi Andrew, agreed, but driver have more incompatible places with constructs like

	AQ_HW_WAIT_FOR(orig_stats_val !=
		       (aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR) &
				       BIT(CAPS_HI_STATISTICS)),
		       1U, 10000U);

For this only the following hack comes to my mind:

	err = readx_poll_timeout_atomic(, val, val, orig_stats_val !=
		             		(aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR) &
			        	BIT(CAPS_HI_STATISTICS)), 1, 10000);

That way it may be better to do the following declaration then:

	#define do_poll_timeout_atomic(cond, sleep_us, timeout_us) \
	({ \
		int _val; /* to make macro happy */
		readx_poll_timeout_atomic(, _val, _val, cond, sleep_us, timeout_us);
	})

Another way would be just to remove AQ_HW_WAIT_FOR macro for all the hard cases
and rewrite it with explicit `for` loop. But that'll reduce readability I guess.

PS

Found duplicating readl_poll_timeout declaration here:
https://elixir.bootlin.com/linux/latest/source/drivers/phy/qualcomm/phy-qcom-ufs-i.h#L27
Not sure what's the reason, but may worth cleaning it up.

Regards,
  Igor