All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Igor Russkikh <Igor.Russkikh@aquantia.com>
Cc: "David S . Miller" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Nikita Danilov <Nikita.Danilov@aquantia.com>
Subject: Re: [PATCH net 5/5] net: aquantia: added err var into AQ_HW_WAIT_FOR construct
Date: Mon, 21 Jan 2019 18:13:23 +0100	[thread overview]
Message-ID: <20190121171323.GK8620@lunn.ch> (raw)
In-Reply-To: <8bae843a7f567eb7187cf7b84372d96e4914b3e7.1547878835.git.igor.russkikh@aquantia.com>

On Mon, Jan 21, 2019 at 02:53:53PM +0000, Igor Russkikh wrote:
> From: Nikita Danilov <nikita.danilov@aquantia.com>
> 
> David noticed this define was hiding 'err' variable
> reference. Thats confusing and counterintuitive.
> 
> Adding err argument explicitly for better visibility
> that err is changed inside macro.
> 
> Signed-off-by: Nikita Danilov <nikita.danilov@aquantia.com>
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
> ---
>  .../ethernet/aquantia/atlantic/aq_hw_utils.h   |  4 ++--
>  .../aquantia/atlantic/hw_atl/hw_atl_a0.c       |  8 ++++----
>  .../aquantia/atlantic/hw_atl/hw_atl_b0.c       |  4 ++--
>  .../aquantia/atlantic/hw_atl/hw_atl_utils.c    | 18 +++++++++---------
>  .../atlantic/hw_atl/hw_atl_utils_fw2x.c        | 10 +++++-----
>  5 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h b/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h
> index dc88a1221f1d..ca1d20d64a39 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h
> @@ -23,7 +23,7 @@
>  
>  #define AQ_HW_SLEEP(_US_) mdelay(_US_)
>  
> -#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.

	     Andrew

  reply	other threads:[~2019-01-21 17:13 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-21 14:53 [PATCH net 0/5] net: aquantia: minor bug fixes after static analysis Igor Russkikh
2019-01-21 14:53 ` [PATCH net 1/5] net: aquantia: fixed memcpy size Igor Russkikh
2019-01-21 14:53 ` [PATCH net 2/5] net: aquantia: added newline at end of file Igor Russkikh
2019-01-21 14:53 ` [PATCH net 3/5] net: aquantia: fixed buffer overflow Igor Russkikh
2019-01-21 14:53 ` [PATCH net 4/5] net: aquantia: fixed instack structure overflow Igor Russkikh
2019-01-21 17:10   ` Andrew Lunn
2019-01-21 14:53 ` [PATCH net 5/5] net: aquantia: added err var into AQ_HW_WAIT_FOR construct Igor Russkikh
2019-01-21 17:13   ` Andrew Lunn [this message]
2019-01-22 12:44     ` Igor Russkikh
2019-01-22 13:34       ` Andrew Lunn
2019-01-23  9:49         ` Igor Russkikh
2019-01-23 13:15           ` Andrew Lunn
2019-01-24 14:28             ` Igor Russkikh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190121171323.GK8620@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=Igor.Russkikh@aquantia.com \
    --cc=Nikita.Danilov@aquantia.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.