From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Lunn <andrew@lunn.ch>
Subject: Re: [PATCH net-next 3/5] net: aquantia: implement WOL support
Date: Wed, 8 Aug 2018 14:16:05 +0200
Message-ID: <20180808121605.GD18314@lunn.ch>
References: <cover.1533650064.git.igor.russkikh@aquantia.com>
 <4b6c2d73c19dac481fbf2656f7a24da90966130b.1533650064.git.igor.russkikh@aquantia.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: "David S . Miller" <davem@davemloft.net>, netdev@vger.kernel.org,
        Yana Esina <yana.esina@aquantia.com>,
        Nikita Danilov <nikita.danilov@aquantia.com>
To: Igor Russkikh <igor.russkikh@aquantia.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from vps0.lunn.ch ([185.16.172.187]:58188 "EHLO vps0.lunn.ch"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1726745AbeHHOfe (ORCPT <rfc822;netdev@vger.kernel.org>);
        Wed, 8 Aug 2018 10:35:34 -0400
Content-Disposition: inline
In-Reply-To: <4b6c2d73c19dac481fbf2656f7a24da90966130b.1533650064.git.igor.russkikh@aquantia.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

> --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> @@ -889,11 +889,13 @@ void aq_nic_deinit(struct aq_nic_s *self)
>  		self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i])
>  		aq_vec_deinit(aq_vec);
>  
> -	if (self->power_state == AQ_HW_POWER_STATE_D0) {
> -		(void)self->aq_fw_ops->deinit(self->aq_hw);
> -	} else {
> -		(void)self->aq_hw_ops->hw_set_power(self->aq_hw,
> -						   self->power_state);
> +	(void)self->aq_fw_ops->deinit(self->aq_hw);

These void casts look a bit ugly. Are they needed? Is the compiler
complaining? If it is complaining, it suggests you should not be
casting anyway...

> +	if (wol_enabled) {
> +		rpc_size = sizeof(prpc->msg_id) + sizeof(prpc->msg_wol);
> +
> +		prpc->msg_id = HAL_ATLANTIC_UTILS_FW_MSG_WOL_ADD;
> +		prpc->msg_wol.priority = 0x10000000; /* normal priority */
> +		prpc->msg_wol.pattern_id = 1U;
> +		prpc->msg_wol.wol_packet_type = 2U; /* Magic Packet */

Maybe add #defines for these magic numbers?

> +
> +		ether_addr_copy((u8 *)&prpc->msg_wol.wol_pattern, mac);
> +	} else {
> +		rpc_size = sizeof(prpc->msg_id) + sizeof(prpc->msg_del_id);
> +
> +		prpc->msg_id = HAL_ATLANTIC_UTILS_FW_MSG_WOL_DEL;
> +		prpc->msg_wol.pattern_id = 1U;
> +	}
> +
> +	err = hw_atl_utils_fw_rpc_call(self, rpc_size);
> +
> +err_exit:
> +	return err;
> +}

  Andrew