From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D8D40C282C0 for ; Wed, 23 Jan 2019 13:15:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9606521848 for ; Wed, 23 Jan 2019 13:15:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b="L6iQyAmG" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726141AbfAWNPG (ORCPT ); Wed, 23 Jan 2019 08:15:06 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:52712 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725991AbfAWNPG (ORCPT ); Wed, 23 Jan 2019 08:15:06 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=6UIkiBPUoA7uKtGyHQVizWW+oUPI2OMPiF++fISMoeY=; b=L6iQyAmGZteBzo1fbGxxclrgl5C1zhg0j9oTSfLtO23vc9aIKtH7++99pAY35gXcBjfdcfNrexCmDi4APnowIzzDVBqlNtFwmCK5JrgKtPwXpMvUHg0wI4giiAKj7O8KnaxShy3Lo8LOu6F7VEZdcJwmZWL8NaCwO8HlN6FlE3U=; Received: from andrew by vps0.lunn.ch with local (Exim 4.84_2) (envelope-from ) id 1gmIMw-0000zo-Id; Wed, 23 Jan 2019 14:15:02 +0100 Date: Wed, 23 Jan 2019 14:15:02 +0100 From: Andrew Lunn To: Igor Russkikh Cc: "David S . Miller" , "netdev@vger.kernel.org" , Nikita Danilov Subject: Re: [PATCH net 5/5] net: aquantia: added err var into AQ_HW_WAIT_FOR construct Message-ID: <20190123131502.GA26413@lunn.ch> References: <8bae843a7f567eb7187cf7b84372d96e4914b3e7.1547878835.git.igor.russkikh@aquantia.com> <20190121171323.GK8620@lunn.ch> <20190122133424.GB3634@lunn.ch> <1bac2c2c-2c60-5043-3b74-0b20138a72c1@aquantia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1bac2c2c-2c60-5043-3b74-0b20138a72c1@aquantia.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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