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
| ||
|
Message-ID: <20190123131502.GA26413@lunn.ch> Date: Wed, 23 Jan 2019 14:15:02 +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 Wed, Jan 23, 2019 at 09:49:25AM +0000, Igor Russkikh wrote: > > > > > 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); You can define a little helper: static u32 aq_hw_read_mpi_state2_addr(struct aq_hw_s *self) { return aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR); } Then readx_poll_timeout(u32 aq_hw_read_mpi_state2_addr, self, stats_val, orig_stats_val != stats_val & BIT(CAPS_HI_STATISTICS)); Given you have the comment: /* Wait FW to report back */ You think the current code is not very readable. So you could actually have: static int sq_fw2_update_stats_fw_wait(aq_hw_s *self, u32 orig_stats_val) { u32 stats_val; return readx_poll_timeout(u32 aq_hw_read_mpi_state2_addr, self, stats_val, orig_stats_val != stats_val & BIT(CAPS_HI_STATISTICS), 1, 1000); } You see this sort of construct in quite a lot of drivers. > 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. Thanks. Andrew
Powered by blists - more mailing lists