[PATCH net-next 0/7] net: atlantic: driver updates

[PATCH net-next 0/7] net: atlantic: driver updates

[Igor Russkikh]

From: Mark Starovoytov <mstarovoitov@marvell.com>

This patch series contains several minor cleanups for the previously
submitted series.

We also add Marvell copyrights on newly touched files.

Mark Starovoytov (7):
  net: atlantic: use __packed instead of the full expansion.
  net: atlantic: minor MACSec code cleanup
  net: atlantic: rename AQ_NIC_RATE_2GS to AQ_NIC_RATE_2G5
  net: atlantic: remove TPO2 check from A0 code
  net: atlantic: remove hw_atl_b0_hw_rss_set call from A2 code
  net: atlantic: remove check for boot code survivability before reset
    request
  net: atlantic: unify get_mac_permanent

 .../ethernet/aquantia/atlantic/aq_common.h    | 11 ++---
 .../ethernet/aquantia/atlantic/aq_ethtool.c   |  2 +-
 .../ethernet/aquantia/atlantic/aq_hw_utils.c  | 41 +++++++++++++++++--
 .../ethernet/aquantia/atlantic/aq_hw_utils.h  |  9 ++--
 .../net/ethernet/aquantia/atlantic/aq_nic.c   | 13 +++---
 .../aquantia/atlantic/hw_atl/hw_atl_a0.c      | 18 ++++----
 .../aquantia/atlantic/hw_atl/hw_atl_b0.c      | 19 +++++----
 .../aquantia/atlantic/hw_atl/hw_atl_b0.h      |  9 ++--
 .../aquantia/atlantic/hw_atl/hw_atl_utils.c   |  9 ++--
 .../aquantia/atlantic/hw_atl/hw_atl_utils.h   |  9 ++--
 .../atlantic/hw_atl/hw_atl_utils_fw2x.c       | 36 ++++------------
 .../aquantia/atlantic/hw_atl2/hw_atl2.c       |  4 +-
 .../aquantia/atlantic/hw_atl2/hw_atl2_utils.c |  8 ----
 .../aquantia/atlantic/hw_atl2/hw_atl2_utils.h |  4 +-
 .../atlantic/hw_atl2/hw_atl2_utils_fw.c       | 26 ++----------
 .../aquantia/atlantic/macsec/macsec_api.c     |  6 +--
 16 files changed, 111 insertions(+), 113 deletions(-)

-- 
2.20.1

[PATCH net-next 1/7] net: atlantic: use __packed instead of the full expansion.

[Igor Russkikh]

From: Mark Starovoytov <mstarovoitov@marvell.com>

This patches fixes the review comment made by Jakub Kicinski
in the "net: atlantic: A2 support" patch series.

Signed-off-by: Mark Starovoytov <mstarovoitov@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 .../net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils.h    | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils.h
index 2317dd8459d0..b66fa346581c 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils.h
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils.h
@@ -103,7 +103,7 @@ struct sleep_proxy_s {
 		u32 crc32;
 	} wake_up_pattern[8];
 
-	struct __attribute__ ((__packed__)) {
+	struct __packed {
 		u8 arp_responder:1;
 		u8 echo_responder:1;
 		u8 igmp_client:1;
@@ -119,7 +119,7 @@ struct sleep_proxy_s {
 	u32 ipv4_offload_addr[8];
 	u32 reserved[8];
 
-	struct __attribute__ ((__packed__)) {
+	struct __packed {
 		u8 ns_responder:1;
 		u8 echo_responder:1;
 		u8 mld_client:1;
-- 
2.20.1

[PATCH net-next 2/7] net: atlantic: minor MACSec code cleanup

[Igor Russkikh]

From: Mark Starovoytov <mstarovoitov@marvell.com>

This patch fixes a couple of minor merge issues found in macsec_api.c
after corresponding patch series has been applied.

These are not real bugs, so pushing to net-next.

Signed-off-by: Mark Starovoytov <mstarovoitov@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 drivers/net/ethernet/aquantia/atlantic/macsec/macsec_api.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/macsec/macsec_api.c b/drivers/net/ethernet/aquantia/atlantic/macsec/macsec_api.c
index fbe9d88b13c7..36c7cf05630a 100644
--- a/drivers/net/ethernet/aquantia/atlantic/macsec/macsec_api.c
+++ b/drivers/net/ethernet/aquantia/atlantic/macsec/macsec_api.c
@@ -846,8 +846,7 @@ static int get_ingress_sakey_record(struct aq_hw_s *hw,
 	rec->key[7] = packed_record[14];
 	rec->key[7] |= packed_record[15] << 16;
 
-	rec->key_len = (rec->key_len & 0xFFFFFFFC) |
-		       (packed_record[16] & 0x3);
+	rec->key_len = packed_record[16] & 0x3;
 
 	return 0;
 }
@@ -1158,6 +1157,7 @@ static int set_egress_ctlf_record(struct aq_hw_s *hw,
 
 	packed_record[0] = rec->sa_da[0] & 0xFFFF;
 	packed_record[1] = (rec->sa_da[0] >> 16) & 0xFFFF;
+
 	packed_record[2] = rec->sa_da[1] & 0xFFFF;
 
 	packed_record[3] = rec->eth_type & 0xFFFF;
@@ -1552,7 +1552,7 @@ static int set_egress_sc_record(struct aq_hw_s *hw,
 
 	packed_record[5] |= (rec->sak_len & 0x3) << 4;
 
-	packed_record[7] |= (rec->valid & 0x1) << 15;
+	packed_record[7] = (rec->valid & 0x1) << 15;
 
 	return set_raw_egress_record(hw, packed_record, 8, 2,
 				     ROWOFFSET_EGRESSSCRECORD + table_index);
-- 
2.20.1

[PATCH net-next 3/7] net: atlantic: rename AQ_NIC_RATE_2GS to AQ_NIC_RATE_2G5

[Igor Russkikh]

From: Mark Starovoytov <mstarovoitov@marvell.com>

This patch changes the constant name to a more logical "2G5"
(for 2.5G speeds).

Signed-off-by: Mark Starovoytov <mstarovoitov@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 .../net/ethernet/aquantia/atlantic/aq_common.h    | 11 ++++++-----
 .../net/ethernet/aquantia/atlantic/aq_ethtool.c   |  2 +-
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c   | 13 +++++++------
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c | 15 ++++++++-------
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c | 15 ++++++++-------
 .../aquantia/atlantic/hw_atl/hw_atl_utils.c       |  9 +++++----
 .../aquantia/atlantic/hw_atl/hw_atl_utils.h       |  9 +++++----
 .../aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c  | 13 +++++++------
 .../ethernet/aquantia/atlantic/hw_atl2/hw_atl2.c  |  2 +-
 .../aquantia/atlantic/hw_atl2/hw_atl2_utils_fw.c  |  2 +-
 10 files changed, 49 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_common.h b/drivers/net/ethernet/aquantia/atlantic/aq_common.h
index 53620ba6d7a6..52ad9433cabc 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_common.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_common.h
@@ -1,7 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * aQuantia Corporation Network Driver
- * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved
+/* Atlantic Network Driver
+ *
+ * Copyright (C) 2014-2019 aQuantia Corporation
+ * Copyright (C) 2019-2020 Marvell International Ltd.
  */
 
 /* File aq_common.h: Basic includes for all files in project. */
@@ -53,14 +54,14 @@
 #define AQ_NIC_RATE_10G		BIT(0)
 #define AQ_NIC_RATE_5G		BIT(1)
 #define AQ_NIC_RATE_5GSR	BIT(2)
-#define AQ_NIC_RATE_2GS		BIT(3)
+#define AQ_NIC_RATE_2G5		BIT(3)
 #define AQ_NIC_RATE_1G		BIT(4)
 #define AQ_NIC_RATE_100M	BIT(5)
 #define AQ_NIC_RATE_10M		BIT(6)
 
 #define AQ_NIC_RATE_EEE_10G	BIT(7)
 #define AQ_NIC_RATE_EEE_5G	BIT(8)
-#define AQ_NIC_RATE_EEE_2GS	BIT(9)
+#define AQ_NIC_RATE_EEE_2G5	BIT(9)
 #define AQ_NIC_RATE_EEE_1G	BIT(10)
 #define AQ_NIC_RATE_EEE_100M	BIT(11)
 
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
index 0c9dd8edc062..86fc77d85fda 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
@@ -605,7 +605,7 @@ static enum hw_atl_fw2x_rate eee_mask_to_ethtool_mask(u32 speed)
 	if (speed & AQ_NIC_RATE_EEE_10G)
 		rate |= SUPPORTED_10000baseT_Full;
 
-	if (speed & AQ_NIC_RATE_EEE_2GS)
+	if (speed & AQ_NIC_RATE_EEE_2G5)
 		rate |= SUPPORTED_2500baseX_Full;
 
 	if (speed & AQ_NIC_RATE_EEE_1G)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index f97b073efd8e..18cad06f2ea7 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0-only
-/*
- * aQuantia Corporation Network Driver
- * Copyright (C) 2014-2019 aQuantia Corporation. All rights reserved
+/* Atlantic Network Driver
+ *
+ * Copyright (C) 2014-2019 aQuantia Corporation
+ * Copyright (C) 2019-2020 Marvell International Ltd.
  */
 
 /* File aq_nic.c: Definition of common code for NIC. */
@@ -894,7 +895,7 @@ void aq_nic_get_link_ksettings(struct aq_nic_s *self,
 		ethtool_link_ksettings_add_link_mode(cmd, supported,
 						     5000baseT_Full);
 
-	if (self->aq_nic_cfg.aq_hw_caps->link_speed_msk & AQ_NIC_RATE_2GS)
+	if (self->aq_nic_cfg.aq_hw_caps->link_speed_msk & AQ_NIC_RATE_2G5)
 		ethtool_link_ksettings_add_link_mode(cmd, supported,
 						     2500baseT_Full);
 
@@ -937,7 +938,7 @@ void aq_nic_get_link_ksettings(struct aq_nic_s *self,
 		ethtool_link_ksettings_add_link_mode(cmd, advertising,
 						     5000baseT_Full);
 
-	if (self->aq_nic_cfg.link_speed_msk  & AQ_NIC_RATE_2GS)
+	if (self->aq_nic_cfg.link_speed_msk  & AQ_NIC_RATE_2G5)
 		ethtool_link_ksettings_add_link_mode(cmd, advertising,
 						     2500baseT_Full);
 
@@ -996,7 +997,7 @@ int aq_nic_set_link_ksettings(struct aq_nic_s *self,
 			break;
 
 		case SPEED_2500:
-			rate = AQ_NIC_RATE_2GS;
+			rate = AQ_NIC_RATE_2G5;
 			break;
 
 		case SPEED_5000:
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
index eee265b4415a..70f06c40bdf2 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0-only
-/*
- * aQuantia Corporation Network Driver
- * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved
+/* Atlantic Network Driver
+ *
+ * Copyright (C) 2014-2019 aQuantia Corporation
+ * Copyright (C) 2019-2020 Marvell International Ltd.
  */
 
 /* File hw_atl_a0.c: Definition of Atlantic hardware specific functions. */
@@ -47,7 +48,7 @@ const struct aq_hw_caps_s hw_atl_a0_caps_aqc100 = {
 	DEFAULT_A0_BOARD_BASIC_CAPABILITIES,
 	.media_type = AQ_HW_MEDIA_TYPE_FIBRE,
 	.link_speed_msk = AQ_NIC_RATE_5G |
-			  AQ_NIC_RATE_2GS |
+			  AQ_NIC_RATE_2G5 |
 			  AQ_NIC_RATE_1G |
 			  AQ_NIC_RATE_100M,
 };
@@ -57,7 +58,7 @@ const struct aq_hw_caps_s hw_atl_a0_caps_aqc107 = {
 	.media_type = AQ_HW_MEDIA_TYPE_TP,
 	.link_speed_msk = AQ_NIC_RATE_10G |
 			  AQ_NIC_RATE_5G |
-			  AQ_NIC_RATE_2GS |
+			  AQ_NIC_RATE_2G5 |
 			  AQ_NIC_RATE_1G |
 			  AQ_NIC_RATE_100M,
 };
@@ -66,7 +67,7 @@ const struct aq_hw_caps_s hw_atl_a0_caps_aqc108 = {
 	DEFAULT_A0_BOARD_BASIC_CAPABILITIES,
 	.media_type = AQ_HW_MEDIA_TYPE_TP,
 	.link_speed_msk = AQ_NIC_RATE_5G |
-			  AQ_NIC_RATE_2GS |
+			  AQ_NIC_RATE_2G5 |
 			  AQ_NIC_RATE_1G |
 			  AQ_NIC_RATE_100M,
 };
@@ -74,7 +75,7 @@ const struct aq_hw_caps_s hw_atl_a0_caps_aqc108 = {
 const struct aq_hw_caps_s hw_atl_a0_caps_aqc109 = {
 	DEFAULT_A0_BOARD_BASIC_CAPABILITIES,
 	.media_type = AQ_HW_MEDIA_TYPE_TP,
-	.link_speed_msk = AQ_NIC_RATE_2GS |
+	.link_speed_msk = AQ_NIC_RATE_2G5 |
 			  AQ_NIC_RATE_1G |
 			  AQ_NIC_RATE_100M,
 };
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index cbb7a00d61b4..1d872547a87c 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0-only
-/*
- * aQuantia Corporation Network Driver
- * Copyright (C) 2014-2019 aQuantia Corporation. All rights reserved
+/* Atlantic Network Driver
+ *
+ * Copyright (C) 2014-2019 aQuantia Corporation
+ * Copyright (C) 2019-2020 Marvell International Ltd.
  */
 
 /* File hw_atl_b0.c: Definition of Atlantic hardware specific functions. */
@@ -59,7 +60,7 @@ const struct aq_hw_caps_s hw_atl_b0_caps_aqc100 = {
 	.media_type = AQ_HW_MEDIA_TYPE_FIBRE,
 	.link_speed_msk = AQ_NIC_RATE_10G |
 			  AQ_NIC_RATE_5G |
-			  AQ_NIC_RATE_2GS |
+			  AQ_NIC_RATE_2G5 |
 			  AQ_NIC_RATE_1G |
 			  AQ_NIC_RATE_100M,
 };
@@ -69,7 +70,7 @@ const struct aq_hw_caps_s hw_atl_b0_caps_aqc107 = {
 	.media_type = AQ_HW_MEDIA_TYPE_TP,
 	.link_speed_msk = AQ_NIC_RATE_10G |
 			  AQ_NIC_RATE_5G |
-			  AQ_NIC_RATE_2GS |
+			  AQ_NIC_RATE_2G5 |
 			  AQ_NIC_RATE_1G |
 			  AQ_NIC_RATE_100M,
 };
@@ -78,7 +79,7 @@ const struct aq_hw_caps_s hw_atl_b0_caps_aqc108 = {
 	DEFAULT_B0_BOARD_BASIC_CAPABILITIES,
 	.media_type = AQ_HW_MEDIA_TYPE_TP,
 	.link_speed_msk = AQ_NIC_RATE_5G |
-			  AQ_NIC_RATE_2GS |
+			  AQ_NIC_RATE_2G5 |
 			  AQ_NIC_RATE_1G |
 			  AQ_NIC_RATE_100M,
 };
@@ -86,7 +87,7 @@ const struct aq_hw_caps_s hw_atl_b0_caps_aqc108 = {
 const struct aq_hw_caps_s hw_atl_b0_caps_aqc109 = {
 	DEFAULT_B0_BOARD_BASIC_CAPABILITIES,
 	.media_type = AQ_HW_MEDIA_TYPE_TP,
-	.link_speed_msk = AQ_NIC_RATE_2GS |
+	.link_speed_msk = AQ_NIC_RATE_2G5 |
 			  AQ_NIC_RATE_1G |
 			  AQ_NIC_RATE_100M,
 };
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
index 1100d40a0302..73c0f41df8d8 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0-only
-/*
- * aQuantia Corporation Network Driver
- * Copyright (C) 2014-2019 aQuantia Corporation. All rights reserved
+/* Atlantic Network Driver
+ *
+ * Copyright (C) 2014-2019 aQuantia Corporation
+ * Copyright (C) 2019-2020 Marvell International Ltd.
  */
 
 /* File hw_atl_utils.c: Definition of common functions for Atlantic hardware
@@ -687,7 +688,7 @@ int hw_atl_utils_mpi_get_link_status(struct aq_hw_s *self)
 			link_status->mbps = 5000U;
 			break;
 
-		case HAL_ATLANTIC_RATE_2GS:
+		case HAL_ATLANTIC_RATE_2G5:
 			link_status->mbps = 2500U;
 			break;
 
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.h
index 99c1b6644ec3..0b4b54fc1de0 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.h
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.h
@@ -1,7 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * aQuantia Corporation Network Driver
- * Copyright (C) 2014-2019 aQuantia Corporation. All rights reserved
+/* Atlantic Network Driver
+ *
+ * Copyright (C) 2014-2019 aQuantia Corporation
+ * Copyright (C) 2019-2020 Marvell International Ltd.
  */
 
 /* File hw_atl_utils.h: Declaration of common functions for Atlantic hardware
@@ -418,7 +419,7 @@ enum hal_atl_utils_fw_state_e {
 #define HAL_ATLANTIC_RATE_10G        BIT(0)
 #define HAL_ATLANTIC_RATE_5G         BIT(1)
 #define HAL_ATLANTIC_RATE_5GSR       BIT(2)
-#define HAL_ATLANTIC_RATE_2GS        BIT(3)
+#define HAL_ATLANTIC_RATE_2G5        BIT(3)
 #define HAL_ATLANTIC_RATE_1G         BIT(4)
 #define HAL_ATLANTIC_RATE_100M       BIT(5)
 #define HAL_ATLANTIC_RATE_INVALID    BIT(6)
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
index 1ad10cc14918..017364486703 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0-only
-/*
- * aQuantia Corporation Network Driver
- * Copyright (C) 2014-2019 aQuantia Corporation. All rights reserved
+/* Atlantic Network Driver
+ *
+ * Copyright (C) 2014-2019 aQuantia Corporation
+ * Copyright (C) 2019-2020 Marvell International Ltd.
  */
 
 /* File hw_atl_utils_fw2x.c: Definition of firmware 2.x functions for
@@ -134,7 +135,7 @@ static enum hw_atl_fw2x_rate link_speed_mask_2fw2x_ratemask(u32 speed)
 	if (speed & AQ_NIC_RATE_5GSR)
 		rate |= FW2X_RATE_5G;
 
-	if (speed & AQ_NIC_RATE_2GS)
+	if (speed & AQ_NIC_RATE_2G5)
 		rate |= FW2X_RATE_2G5;
 
 	if (speed & AQ_NIC_RATE_1G)
@@ -155,7 +156,7 @@ static u32 fw2x_to_eee_mask(u32 speed)
 	if (speed & HW_ATL_FW2X_CAP_EEE_5G_MASK)
 		rate |= AQ_NIC_RATE_EEE_5G;
 	if (speed & HW_ATL_FW2X_CAP_EEE_2G5_MASK)
-		rate |= AQ_NIC_RATE_EEE_2GS;
+		rate |= AQ_NIC_RATE_EEE_2G5;
 	if (speed & HW_ATL_FW2X_CAP_EEE_1G_MASK)
 		rate |= AQ_NIC_RATE_EEE_1G;
 
@@ -170,7 +171,7 @@ static u32 eee_mask_to_fw2x(u32 speed)
 		rate |= HW_ATL_FW2X_CAP_EEE_10G_MASK;
 	if (speed & AQ_NIC_RATE_EEE_5G)
 		rate |= HW_ATL_FW2X_CAP_EEE_5G_MASK;
-	if (speed & AQ_NIC_RATE_EEE_2GS)
+	if (speed & AQ_NIC_RATE_EEE_2G5)
 		rate |= HW_ATL_FW2X_CAP_EEE_2G5_MASK;
 	if (speed & AQ_NIC_RATE_EEE_1G)
 		rate |= HW_ATL_FW2X_CAP_EEE_1G_MASK;
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2.c
index 04d194f754fa..84d9b828dc4e 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2.c
@@ -60,7 +60,7 @@ const struct aq_hw_caps_s hw_atl2_caps_aqc113 = {
 	.media_type = AQ_HW_MEDIA_TYPE_TP,
 	.link_speed_msk = AQ_NIC_RATE_10G |
 			  AQ_NIC_RATE_5G  |
-			  AQ_NIC_RATE_2GS |
+			  AQ_NIC_RATE_2G5 |
 			  AQ_NIC_RATE_1G  |
 			  AQ_NIC_RATE_100M      |
 			  AQ_NIC_RATE_10M,
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils_fw.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils_fw.c
index f5fb4b11f51a..e8f4aad8c1e5 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils_fw.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils_fw.c
@@ -129,7 +129,7 @@ static void a2_link_speed_mask2fw(u32 speed,
 	link_options->rate_10G = !!(speed & AQ_NIC_RATE_10G);
 	link_options->rate_5G = !!(speed & AQ_NIC_RATE_5G);
 	link_options->rate_N5G = !!(speed & AQ_NIC_RATE_5GSR);
-	link_options->rate_2P5G = !!(speed & AQ_NIC_RATE_2GS);
+	link_options->rate_2P5G = !!(speed & AQ_NIC_RATE_2G5);
 	link_options->rate_N2P5G = link_options->rate_2P5G;
 	link_options->rate_1G = !!(speed & AQ_NIC_RATE_1G);
 	link_options->rate_100M = !!(speed & AQ_NIC_RATE_100M);
-- 
2.20.1

[PATCH net-next 4/7] net: atlantic: remove TPO2 check from A0 code

[Igor Russkikh]

From: Mark Starovoytov <mstarovoitov@marvell.com>

TPO2 was introduced in B0 only, no reason to check for it in A0 code.

Signed-off-by: Mark Starovoytov <mstarovoitov@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
index 70f06c40bdf2..1b0670a8ae33 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
@@ -268,8 +268,7 @@ static int hw_atl_a0_hw_init_tx_path(struct aq_hw_s *self)
 	hw_atl_tdm_tx_desc_wr_wb_irq_en_set(self, 1U);
 
 	/* misc */
-	aq_hw_write_reg(self, 0x00007040U, ATL_HW_IS_CHIP_FEATURE(self, TPO2) ?
-			0x00010000U : 0x00000000U);
+	aq_hw_write_reg(self, 0x00007040U, 0x00000000U);
 	hw_atl_tdm_tx_dca_en_set(self, 0U);
 	hw_atl_tdm_tx_dca_mode_set(self, 0U);
 
-- 
2.20.1

[PATCH net-next 5/7] net: atlantic: remove hw_atl_b0_hw_rss_set call from A2 code

[Igor Russkikh]

From: Mark Starovoytov <mstarovoitov@marvell.com>

No need to call hw_atl_b0_hw_rss_set from hw_atl2_hw_rss_set

Signed-off-by: Mark Starovoytov <mstarovoitov@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 .../net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c    | 4 ++--
 .../net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.h    | 9 ++++-----
 drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2.c | 2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index 1d872547a87c..fa3cd7e9954b 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -216,8 +216,8 @@ int hw_atl_b0_hw_rss_hash_set(struct aq_hw_s *self,
 	return err;
 }
 
-int hw_atl_b0_hw_rss_set(struct aq_hw_s *self,
-			 struct aq_rss_parameters *rss_params)
+static int hw_atl_b0_hw_rss_set(struct aq_hw_s *self,
+				struct aq_rss_parameters *rss_params)
 {
 	u32 num_rss_queues = max(1U, self->aq_nic_cfg->num_rss_queues);
 	u8 *indirection_table =	rss_params->indirection_table;
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.h
index f5091d79ab43..b855459272ca 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.h
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.h
@@ -1,7 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * aQuantia Corporation Network Driver
- * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved
+/* Atlantic Network Driver
+ *
+ * Copyright (C) 2014-2019 aQuantia Corporation
+ * Copyright (C) 2019-2020 Marvell International Ltd.
  */
 
 /* File hw_atl_b0.h: Declaration of abstract interface for Atlantic hardware
@@ -35,8 +36,6 @@ extern const struct aq_hw_ops hw_atl_ops_b0;
 
 int hw_atl_b0_hw_rss_hash_set(struct aq_hw_s *self,
 			      struct aq_rss_parameters *rss_params);
-int hw_atl_b0_hw_rss_set(struct aq_hw_s *self,
-			 struct aq_rss_parameters *rss_params);
 int hw_atl_b0_hw_offload_set(struct aq_hw_s *self,
 			     struct aq_nic_cfg_s *aq_nic_cfg);
 
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2.c
index 84d9b828dc4e..6f2b33ae3d06 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2.c
@@ -172,7 +172,7 @@ static int hw_atl2_hw_rss_set(struct aq_hw_s *self,
 	for (i = HW_ATL2_RSS_REDIRECTION_MAX; i--;)
 		hw_atl2_new_rpf_rss_redir_set(self, 0, i, indirection_table[i]);
 
-	return hw_atl_b0_hw_rss_set(self, rss_params);
+	return aq_hw_err_from_flags(self);
 }
 
 static int hw_atl2_hw_init_tx_path(struct aq_hw_s *self)
-- 
2.20.1

[PATCH net-next 6/7] net: atlantic: remove check for boot code survivability before reset request

[Igor Russkikh]

From: Mark Starovoytov <mstarovoitov@marvell.com>

This patch removes unnecessary check for boot code survivability before
reset request.

Signed-off-by: Mark Starovoytov <mstarovoitov@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 .../ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils.c    | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils.c
index 85ccc9a011a0..f3766780e975 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils.c
@@ -75,14 +75,6 @@ int hw_atl2_utils_soft_reset(struct aq_hw_s *self)
 	u32 rbl_request;
 	int err;
 
-	err = readx_poll_timeout_atomic(hw_atl2_mif_mcp_boot_reg_get, self,
-				rbl_status,
-				((rbl_status & AQ_A2_BOOT_STARTED) &&
-				 (rbl_status != 0xFFFFFFFFu)),
-				10, 500000);
-	if (err)
-		aq_pr_trace("Boot code probably hanged, reboot anyway");
-
 	hw_atl2_mif_host_req_int_clr(self, 0x01);
 	rbl_request = AQ_A2_FW_BOOT_REQ_REBOOT;
 #ifdef AQ_CFG_FAST_START
-- 
2.20.1

[PATCH net-next 7/7] net: atlantic: unify get_mac_permanent

[Igor Russkikh]

From: Mark Starovoytov <mstarovoitov@marvell.com>

MAC generation in case if MAC is not populated is the same for both A1 and
A2.
This patch moves MAC generation into a separate function, which is called
from both places to reduce the amount of copy/paste.

Signed-off-by: Mark Starovoytov <mstarovoitov@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 .../ethernet/aquantia/atlantic/aq_hw_utils.c  | 41 +++++++++++++++++--
 .../ethernet/aquantia/atlantic/aq_hw_utils.h  |  9 ++--
 .../atlantic/hw_atl/hw_atl_utils_fw2x.c       | 23 +----------
 .../atlantic/hw_atl2/hw_atl2_utils_fw.c       | 24 ++---------
 4 files changed, 49 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.c b/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.c
index 7dbf49adcea6..0bc01772ead2 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.c
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0-only
-/*
- * aQuantia Corporation Network Driver
- * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved
+/* Atlantic Network Driver
+ *
+ * Copyright (C) 2014-2019 aQuantia Corporation
+ * Copyright (C) 2019-2020 Marvell International Ltd.
  */
 
 /* File aq_hw_utils.c: Definitions of helper functions used across
@@ -79,3 +80,37 @@ int aq_hw_err_from_flags(struct aq_hw_s *hw)
 err_exit:
 	return err;
 }
+
+static inline bool aq_hw_is_zero_ether_addr(const u8 *addr)
+{
+	return (*(const u16 *)(addr) | addr[2]) == 0;
+}
+
+bool aq_hw_is_valid_ether_addr(const u8 *addr)
+{
+	return !is_multicast_ether_addr(addr) &&
+	       !aq_hw_is_zero_ether_addr(addr);
+}
+
+void aq_hw_eth_random_addr(u8 *mac)
+{
+	unsigned int rnd = 0;
+	u32 h = 0U;
+	u32 l = 0U;
+
+	get_random_bytes(&rnd, sizeof(unsigned int));
+
+	l = 0xE3000000U | (0xFFFFU & rnd) | (0x00 << 16);
+	h = 0x8001300EU;
+
+	mac[5] = (u8)(0xFFU & l);
+	l >>= 8;
+	mac[4] = (u8)(0xFFU & l);
+	l >>= 8;
+	mac[3] = (u8)(0xFFU & l);
+	l >>= 8;
+	mac[2] = (u8)(0xFFU & l);
+	mac[1] = (u8)(0xFFU & h);
+	h >>= 8;
+	mac[0] = (u8)(0xFFU & h);
+}
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h b/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h
index 9ef82d487e01..4753e3e301a3 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h
@@ -1,7 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * aQuantia Corporation Network Driver
- * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved
+/* Atlantic Network Driver
+ *
+ * Copyright (C) 2014-2019 aQuantia Corporation
+ * Copyright (C) 2019-2020 Marvell International Ltd.
  */
 
 /* File aq_hw_utils.h: Declaration of helper functions used across hardware
@@ -34,5 +35,7 @@ u32 aq_hw_read_reg(struct aq_hw_s *hw, u32 reg);
 void aq_hw_write_reg(struct aq_hw_s *hw, u32 reg, u32 value);
 u64 aq_hw_read_reg64(struct aq_hw_s *hw, u32 reg);
 int aq_hw_err_from_flags(struct aq_hw_s *hw);
+bool aq_hw_is_valid_ether_addr(const u8 *addr);
+void aq_hw_eth_random_addr(u8 *addr);
 
 #endif /* AQ_HW_UTILS_H */
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
index 017364486703..8a09e86b3990 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -283,8 +283,6 @@ static int aq_fw2x_get_mac_permanent(struct aq_hw_s *self, u8 *mac)
 	u32 efuse_addr = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_EFUSE_ADDR);
 	u32 mac_addr[2] = { 0 };
 	int err = 0;
-	u32 h = 0U;
-	u32 l = 0U;
 
 	if (efuse_addr != 0) {
 		err = hw_atl_utils_fw_downld_dwords(self,
@@ -299,25 +297,8 @@ static int aq_fw2x_get_mac_permanent(struct aq_hw_s *self, u8 *mac)
 
 	ether_addr_copy(mac, (u8 *)mac_addr);
 
-	if ((mac[0] & 0x01U) || ((mac[0] | mac[1] | mac[2]) == 0x00U)) {
-		unsigned int rnd = 0;
-
-		get_random_bytes(&rnd, sizeof(unsigned int));
-
-		l = 0xE3000000U | (0xFFFFU & rnd) | (0x00 << 16);
-		h = 0x8001300EU;
-
-		mac[5] = (u8)(0xFFU & l);
-		l >>= 8;
-		mac[4] = (u8)(0xFFU & l);
-		l >>= 8;
-		mac[3] = (u8)(0xFFU & l);
-		l >>= 8;
-		mac[2] = (u8)(0xFFU & l);
-		mac[1] = (u8)(0xFFU & h);
-		h >>= 8;
-		mac[0] = (u8)(0xFFU & h);
-	}
+	if (!aq_hw_is_valid_ether_addr(mac))
+		aq_hw_eth_random_addr(mac);
 
 	return err;
 }
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils_fw.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils_fw.c
index e8f4aad8c1e5..87f1133e6c39 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils_fw.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl2/hw_atl2_utils_fw.c
@@ -6,6 +6,7 @@
 #include <linux/iopoll.h>
 
 #include "aq_hw.h"
+#include "aq_hw_utils.h"
 #include "hw_atl/hw_atl_llh.h"
 #include "hw_atl2_utils.h"
 #include "hw_atl2_llh.h"
@@ -212,27 +213,8 @@ static int aq_a2_fw_get_mac_permanent(struct aq_hw_s *self, u8 *mac)
 	hw_atl2_shared_buffer_get(self, mac_address, mac_address);
 	ether_addr_copy(mac, (u8 *)mac_address.aligned.mac_address);
 
-	if ((mac[0] & 0x01U) || ((mac[0] | mac[1] | mac[2]) == 0x00U)) {
-		unsigned int rnd = 0;
-		u32 h;
-		u32 l;
-
-		get_random_bytes(&rnd, sizeof(unsigned int));
-
-		l = 0xE3000000U | (0xFFFFU & rnd) | (0x00 << 16);
-		h = 0x8001300EU;
-
-		mac[5] = (u8)(0xFFU & l);
-		l >>= 8;
-		mac[4] = (u8)(0xFFU & l);
-		l >>= 8;
-		mac[3] = (u8)(0xFFU & l);
-		l >>= 8;
-		mac[2] = (u8)(0xFFU & l);
-		mac[1] = (u8)(0xFFU & h);
-		h >>= 8;
-		mac[0] = (u8)(0xFFU & h);
-	}
+	if (!aq_hw_is_valid_ether_addr(mac))
+		aq_hw_eth_random_addr(mac);
 
 	return 0;
 }
-- 
2.20.1

Re: [PATCH net-next 7/7] net: atlantic: unify get_mac_permanent

[Jakub Kicinski]

On Thu, 7 May 2020 11:15:10 +0300 Igor Russkikh wrote:
> From: Mark Starovoytov <mstarovoitov@marvell.com>
> 
> MAC generation in case if MAC is not populated is the same for both A1 and
> A2.
> This patch moves MAC generation into a separate function, which is called
> from both places to reduce the amount of copy/paste.

Right, but why do you have your own mac generation rather than using
eth_hw_addr_random(). You need to set NET_ADDR_RANDOM for example,
just use standard helpers, please.

> Signed-off-by: Mark Starovoytov <mstarovoitov@marvell.com>
> Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
> ---
>  .../ethernet/aquantia/atlantic/aq_hw_utils.c  | 41 +++++++++++++++++--
>  .../ethernet/aquantia/atlantic/aq_hw_utils.h  |  9 ++--
>  .../atlantic/hw_atl/hw_atl_utils_fw2x.c       | 23 +----------
>  .../atlantic/hw_atl2/hw_atl2_utils_fw.c       | 24 ++---------
>  4 files changed, 49 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.c b/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.c
> index 7dbf49adcea6..0bc01772ead2 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.c
> @@ -1,7 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * aQuantia Corporation Network Driver
> - * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved
> +/* Atlantic Network Driver
> + *
> + * Copyright (C) 2014-2019 aQuantia Corporation
> + * Copyright (C) 2019-2020 Marvell International Ltd.
>   */
>  
>  /* File aq_hw_utils.c: Definitions of helper functions used across
> @@ -79,3 +80,37 @@ int aq_hw_err_from_flags(struct aq_hw_s *hw)
>  err_exit:
>  	return err;
>  }
> +
> +static inline bool aq_hw_is_zero_ether_addr(const u8 *addr)

No static inlines in C files, please. Let compiler decide inlineing &
generate a warning when function becomes unused.

> +{
> +	return (*(const u16 *)(addr) | addr[2]) == 0;

It's probably fine in practice but the potentially u16 read is entirely
unnecessary. This is not performance sensitive code.

> +}
> +
> +bool aq_hw_is_valid_ether_addr(const u8 *addr)
> +{
> +	return !is_multicast_ether_addr(addr) &&
> +	       !aq_hw_is_zero_ether_addr(addr);
> +}
> +
> +void aq_hw_eth_random_addr(u8 *mac)
> +{
> +	unsigned int rnd = 0;
> +	u32 h = 0U;
> +	u32 l = 0U;
> +
> +	get_random_bytes(&rnd, sizeof(unsigned int));
> +	l = 0xE300 0000U | (0xFFFFU & rnd) | (0x00 << 16);
> +	h = 0x8001300EU;
> +
> +	mac[5] = (u8)(0xFFU & l);
> +	l >>= 8;
> +	mac[4] = (u8)(0xFFU & l);
> +	l >>= 8;
> +	mac[3] = (u8)(0xFFU & l);
> +	l >>= 8;
> +	mac[2] = (u8)(0xFFU & l);
> +	mac[1] = (u8)(0xFFU & h);
> +	h >>= 8;
> +	mac[0] = (u8)(0xFFU & h);

This can be greatly simplified using helpers from etherdevice.h, if
it's really needed.

> +}

Re: [PATCH net-next 7/7] net: atlantic: unify get_mac_permanent

[Igor Russkikh]

Hi Jakub,


>> This patch moves MAC generation into a separate function, which is
> called
>> from both places to reduce the amount of copy/paste.
> 
> Right, but why do you have your own mac generation rather than using
> eth_hw_addr_random(). You need to set NET_ADDR_RANDOM for example,
> just use standard helpers, please.

We want this still be an Aquantia vendor id MAC, not a fully random mac.
Thats why the logic below randomizes only low three octets.


>>  }
>> +
>> +static inline bool aq_hw_is_zero_ether_addr(const u8 *addr)
> 
> No static inlines in C files, please. Let compiler decide inlineing &
> generate a warning when function becomes unused.

Ok, will fix.

>> +	get_random_bytes(&rnd, sizeof(unsigned int));
>> +	l = 0xE300 0000U | (0xFFFFU & rnd) | (0x00 << 16);
>> +	h = 0x8001300EU;
>> +
>> +	mac[5] = (u8)(0xFFU & l);
>> +	l >>= 8;
>> +	mac[4] = (u8)(0xFFU & l);
>> +	l >>= 8;
>> +	mac[3] = (u8)(0xFFU & l);
>> +	l >>= 8;
>> +	mac[2] = (u8)(0xFFU & l);
>> +	mac[1] = (u8)(0xFFU & h);
>> +	h >>= 8;
>> +	mac[0] = (u8)(0xFFU & h);
> 
> This can be greatly simplified using helpers from etherdevice.h, if
> it's really needed.

This is the exact place where we put Aquantia vendor id, even if mac is random.

eth_hw_addr_random is more suitable for software devices like bridges, which
are not related to any vendor.

Regards,
  Igor

Re: [PATCH net-next 7/7] net: atlantic: unify get_mac_permanent

[Andrew Lunn]

On Fri, May 08, 2020 at 12:14:51PM +0300, Igor Russkikh wrote:
> Hi Jakub,
> 
> 
> >> This patch moves MAC generation into a separate function, which is
> > called
> >> from both places to reduce the amount of copy/paste.
> > 
> > Right, but why do you have your own mac generation rather than using
> > eth_hw_addr_random(). You need to set NET_ADDR_RANDOM for example,
> > just use standard helpers, please.
> 
> We want this still be an Aquantia vendor id MAC, not a fully random mac.
> Thats why the logic below randomizes only low three octets.

Hi Igor

How safe is that?  It reduces the available pool space by 22
bits. It greatly increases the likelihood of a collision.

> 
> >> +	get_random_bytes(&rnd, sizeof(unsigned int));
> >> +	l = 0xE300 0000U | (0xFFFFU & rnd) | (0x00 << 16);
> >> +	h = 0x8001300EU;

Is this Marvell/Aquantias OUI? Are you setting the locally
administered bit? You probably should be, since this is local, not
issued with a guarantee of being unique. 

       Andrew

Re: [EXT] Re: [PATCH net-next 7/7] net: atlantic: unify get_mac_permanent

[Igor Russkikh]

>>> Right, but why do you have your own mac generation rather than using
>>> eth_hw_addr_random(). You need to set NET_ADDR_RANDOM for example,
>>> just use standard helpers, please.
>>
>> We want this still be an Aquantia vendor id MAC, not a fully random mac.
>> Thats why the logic below randomizes only low three octets.
> 
> Hi Igor
> 
> How safe is that?  It reduces the available pool space by 22
> bits. It greatly increases the likelihood of a collision.

>>>> +	get_random_bytes(&rnd, sizeof(unsigned int));
>>>> +	l = 0xE300 0000U | (0xFFFFU & rnd) | (0x00 << 16);
>>>> +	h = 0x8001300EU;
> 
> Is this Marvell/Aquantias OUI? Are you setting the locally
> administered bit? You probably should be, since this is local, not
> issued with a guarantee of being unique. 

Yes, thats Aquantia's ID: 300EE3

Honestly, the subject of the discussion are only adapters with zeroed, not
burned MACs. In production there could not exist such adapters. We do have
this code mainly to cover engineering samples some of which comes unflashed.

So overall, I feel its abit overkill to care about collisions.
But we still like to see our engineering samples to have our OUI for ease of
scripting and maintenance.

Regards,
  Igor

Re: [EXT] Re: [PATCH net-next 7/7] net: atlantic: unify get_mac_permanent

[Andrew Lunn]

On Fri, May 08, 2020 at 04:22:40PM +0300, Igor Russkikh wrote:
> 
> >>> Right, but why do you have your own mac generation rather than using
> >>> eth_hw_addr_random(). You need to set NET_ADDR_RANDOM for example,
> >>> just use standard helpers, please.
> >>
> >> We want this still be an Aquantia vendor id MAC, not a fully random mac.
> >> Thats why the logic below randomizes only low three octets.
> > 
> > Hi Igor
> > 
> > How safe is that?  It reduces the available pool space by 22
> > bits. It greatly increases the likelihood of a collision.
> 
> >>>> +	get_random_bytes(&rnd, sizeof(unsigned int));
> >>>> +	l = 0xE300 0000U | (0xFFFFU & rnd) | (0x00 << 16);
> >>>> +	h = 0x8001300EU;
> > 
> > Is this Marvell/Aquantias OUI? Are you setting the locally
> > administered bit? You probably should be, since this is local, not
> > issued with a guarantee of being unique. 
> 
> Yes, thats Aquantia's ID: 300EE3
> 
> Honestly, the subject of the discussion are only adapters with zeroed, not
> burned MACs. In production there could not exist such adapters. We do have
> this code mainly to cover engineering samples some of which comes unflashed.
> 
> So overall, I feel its abit overkill to care about collisions.
> But we still like to see our engineering samples to have our OUI for ease of
> scripting and maintenance.

Hi Igor

At minimum, you need to put this as a comment.

And since it is not supposed to happen, you might want to throw a
WARN_ON(). The fact you are somewhat hiding the problem the FLASH is
empty, makes it more likely you actually ship unflashed devices to the
customers. A big scary looking kernel stack trace should swing the
risk back towards the safer side, and if manufacturing does mess up,
you are likely to get feedback from customers pretty quickly.

     Andrew

Re: [PATCH net-next 7/7] net: atlantic: unify get_mac_permanent

[Jakub Kicinski]

On Fri, 8 May 2020 12:14:51 +0300 Igor Russkikh wrote:
> >> This patch moves MAC generation into a separate function, which is called  
> >> from both places to reduce the amount of copy/paste.  
> > 
> > Right, but why do you have your own mac generation rather than using
> > eth_hw_addr_random(). You need to set NET_ADDR_RANDOM for example,
> > just use standard helpers, please.  
> 
> We want this still be an Aquantia vendor id MAC, not a fully random mac.
> Thats why the logic below randomizes only low three octets.

Are there any other drivers in tree which do that? I think the standard
operating procedure is:

if (!valid) {
	netdev_warn(dev, "Invalid MAC using random\n");
	eth_hw_addr_random(dev);
}

Please see all the eth_hw_addr_random() calls in drivers.

> >> +static inline bool aq_hw_is_zero_ether_addr(const u8 *addr)  
> > 
> > No static inlines in C files, please. Let compiler decide inlineing &
> > generate a warning when function becomes unused.  
> 
> Ok, will fix.
> 
> >> +	get_random_bytes(&rnd, sizeof(unsigned int));
> >> +	l = 0xE300 0000U | (0xFFFFU & rnd) | (0x00 << 16);
> >> +	h = 0x8001300EU;
> >> +
> >> +	mac[5] = (u8)(0xFFU & l);
> >> +	l >>= 8;
> >> +	mac[4] = (u8)(0xFFU & l);
> >> +	l >>= 8;
> >> +	mac[3] = (u8)(0xFFU & l);
> >> +	l >>= 8;
> >> +	mac[2] = (u8)(0xFFU & l);
> >> +	mac[1] = (u8)(0xFFU & h);
> >> +	h >>= 8;
> >> +	mac[0] = (u8)(0xFFU & h);  
> > 
> > This can be greatly simplified using helpers from etherdevice.h, if
> > it's really needed.  
> 
> This is the exact place where we put Aquantia vendor id, even if mac is random.
> 
> eth_hw_addr_random is more suitable for software devices like bridges, which
> are not related to any vendor.

What's the basis for this statement? 

Please look around the existing drivers.

Re: [PATCH net-next 7/7] net: atlantic: unify get_mac_permanent

[Igor Russkikh]

>> Thats why the logic below randomizes only low three octets.
> 
> Are there any other drivers in tree which do that? I think the standard
> operating procedure is:
> 
> if (!valid) {
> 	netdev_warn(dev, "Invalid MAC using random\n");
> 	eth_hw_addr_random(dev);
> }
> 
> Please see all the eth_hw_addr_random() calls in drivers.

You are right indeed. Sorry I somehow overlooked this sequence in other device
drivers. Agree, its better to unify behavior then.

Thanks,
  Igor