netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ixgbe: Add support for firmware update
@ 2024-06-09  8:57 Richard chien
  2024-06-09 11:23 ` Markus Elfring
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Richard chien @ 2024-06-09  8:57 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni
  Cc: jesse.brandeburg, anthony.l.nguyen, intel-wired-lan, netdev,
	linux-kernel, Richard chien

This patch adds support for firmware update to the in-tree ixgbe driver and it is actually a port
from the out-of-tree ixgbe driver. In-band firmware update is one of the essential system maintenance
tasks. To simplify this task, the Intel online firmware update utility provides a common interface
that works across different Intel NICs running the igb/ixgbe/i40e drivers. Unfortunately, the in-tree
igb and ixgbe drivers are unable to support this firmware update utility, causing problems for
enterprise users who do not or cannot use out-of-distro drivers due to security and various other
reasons (e.g. commercial Linux distros do not provide technical support for out-of-distro drivers).
As a result, getting this feature into the in-tree ixgbe driver is highly desirable.

Signed-off-by: Richard chien <richard.chien@hpe.com>
---
 .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  | 360 +++++++++++++-----
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  11 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |  37 ++
 3 files changed, 317 insertions(+), 91 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 6e6e6f184..3ce5c662a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -993,114 +993,292 @@ static void ixgbe_get_regs(struct net_device *netdev,
 
 static int ixgbe_get_eeprom_len(struct net_device *netdev)
 {
-	struct ixgbe_adapter *adapter = netdev_priv(netdev);
-	return adapter->hw.eeprom.word_size * 2;
+        struct ixgbe_adapter *adapter = netdev_priv(netdev);
+
+        return pci_resource_len(adapter->pdev, 0);
+}
+
+static u8 ixgbe_nvmupd_get_module(u32 val)
+{
+        return (u8)(val & IXGBE_NVMUPD_MOD_PNT_MASK);
+}
+
+static int ixgbe_nvmupd_validate_offset(struct ixgbe_adapter *adapter,
+                                        u32 offset)
+{
+        struct net_device *netdev = adapter->netdev;
+
+        switch (offset) {
+        case IXGBE_STATUS:
+        case IXGBE_ESDP:
+        case IXGBE_MSCA:
+        case IXGBE_MSRWD:
+        case IXGBE_EEC_8259X:
+        case IXGBE_FLA_8259X:
+        case IXGBE_FLOP:
+        case IXGBE_SWSM_8259X:
+        case IXGBE_FWSM_8259X:
+        case IXGBE_FACTPS_8259X:
+        case IXGBE_GSSR:
+        case IXGBE_HICR:
+        case IXGBE_FWSTS:
+                return 0;
+        default:
+                if ((offset >= IXGBE_MAVTV(0) && offset <= IXGBE_MAVTV(7)) ||
+                    (offset >= IXGBE_RAL(0) && offset <= IXGBE_RAH(15)))
+                        return 0;
+        }
+
+        switch (adapter->hw.mac.type) {
+        case ixgbe_mac_82599EB:
+                switch (offset) {
+                case IXGBE_AUTOC:
+                case IXGBE_EERD:
+                case IXGBE_BARCTRL:
+                        return 0;
+                default:
+                        if (offset >= 0x00020000 &&
+                            offset <= ixgbe_get_eeprom_len(netdev))
+                                return 0;
+                }
+                break;
+        case ixgbe_mac_X540:
+                switch (offset) {
+                case IXGBE_EERD:
+                case IXGBE_EEWR:
+                case IXGBE_SRAMREL:
+                case IXGBE_BARCTRL:
+                        return 0;
+                default:
+                        if ((offset >= 0x00020000 &&
+                             offset <= ixgbe_get_eeprom_len(netdev)))
+                                return 0;
+                }
+                break;
+        case ixgbe_mac_X550:
+                switch (offset) {
+                case IXGBE_EEWR:
+                case IXGBE_SRAMREL:
+                case IXGBE_PHYCTL_82599:
+                case IXGBE_FWRESETCNT:
+                        return 0;
+                default:
+                        if (offset >= IXGBE_FLEX_MNG_PTR(0) &&
+                            offset <= IXGBE_FLEX_MNG_PTR(447))
+                                return 0;
+                }
+                break;
+        case ixgbe_mac_X550EM_x:
+                switch (offset) {
+                case IXGBE_PHYCTL_82599:
+                case IXGBE_NW_MNG_IF_SEL:
+                case IXGBE_FWRESETCNT:
+                case IXGBE_I2CCTL_X550:
+                        return 0;
+                default:
+                        if ((offset >= IXGBE_FLEX_MNG_PTR(0) &&
+                             offset <= IXGBE_FLEX_MNG_PTR(447)) ||
+                            (offset >= IXGBE_FUSES0_GROUP(0) &&
+                             offset <= IXGBE_FUSES0_GROUP(7)))
+                                return 0;
+                }
+                break;
+        case ixgbe_mac_x550em_a:
+                switch (offset) {
+                case IXGBE_PHYCTL_82599:
+                case IXGBE_NW_MNG_IF_SEL:
+                case IXGBE_FWRESETCNT:
+                case IXGBE_I2CCTL_X550:
+                case IXGBE_FLA_X550EM_a:
+                case IXGBE_SWSM_X550EM_a:
+                case IXGBE_FWSM_X550EM_a:
+                case IXGBE_SWFW_SYNC_X550EM_a:
+                case IXGBE_FACTPS_X550EM_a:
+                case IXGBE_EEC_X550EM_a:
+                        return 0;
+                default:
+                        if (offset >= IXGBE_FLEX_MNG_PTR(0) &&
+                            offset <= IXGBE_FLEX_MNG_PTR(447))
+                                return 0;
+                }
+        default:
+                break;
+        }
+
+        return -ENOTTY;
+}
+
+static int ixgbe_nvmupd_command(struct ixgbe_hw *hw,
+                                struct ixgbe_nvm_access *nvm,
+                                u8 *bytes)
+{
+        u32 command;
+        int ret_val = 0;
+        u8 module;
+
+        command = nvm->command;
+        module = ixgbe_nvmupd_get_module(nvm->config);
+
+        switch (command) {
+        case IXGBE_NVMUPD_CMD_REG_READ:
+                switch (module) {
+                case IXGBE_NVMUPD_EXEC_FEATURES:
+                        if (nvm->data_size == hw->nvmupd_features.size)
+                                memcpy(bytes, &hw->nvmupd_features,
+                                       hw->nvmupd_features.size);
+                        else
+                                ret_val = -ENOMEM;
+                break;
+                default:
+                        if (ixgbe_nvmupd_validate_offset(hw->back, nvm->offset))
+                                return -ENOTTY;
+
+                        if (nvm->data_size == 1)
+                                *((u8 *)bytes) = IXGBE_R8_Q(hw, nvm->offset);
+                        else
+                                *((u32 *)bytes) = IXGBE_R32_Q(hw, nvm->offset);
+                break;
+                }
+        break;
+        case IXGBE_NVMUPD_CMD_REG_WRITE:
+                if (ixgbe_nvmupd_validate_offset(hw->back, nvm->offset))
+                        return -ENOTTY;
+
+                IXGBE_WRITE_REG(hw, nvm->offset, *((u32 *)bytes));
+        break;
+        }
+
+        return ret_val;
 }
 
 static int ixgbe_get_eeprom(struct net_device *netdev,
-			    struct ethtool_eeprom *eeprom, u8 *bytes)
+                            struct ethtool_eeprom *eeprom, u8 *bytes)
 {
-	struct ixgbe_adapter *adapter = netdev_priv(netdev);
-	struct ixgbe_hw *hw = &adapter->hw;
-	u16 *eeprom_buff;
-	int first_word, last_word, eeprom_len;
-	int ret_val = 0;
-	u16 i;
+        struct ixgbe_adapter *adapter = netdev_priv(netdev);
+        struct ixgbe_hw *hw = &adapter->hw;
+        u16 *eeprom_buff;
+        int first_word, last_word, eeprom_len;
+        struct ixgbe_nvm_access *nvm;
+        u32 magic;
+        int ret_val = 0;
+        u16 i;
 
-	if (eeprom->len == 0)
-		return -EINVAL;
-
-	eeprom->magic = hw->vendor_id | (hw->device_id << 16);
-
-	first_word = eeprom->offset >> 1;
-	last_word = (eeprom->offset + eeprom->len - 1) >> 1;
-	eeprom_len = last_word - first_word + 1;
-
-	eeprom_buff = kmalloc_array(eeprom_len, sizeof(u16), GFP_KERNEL);
-	if (!eeprom_buff)
-		return -ENOMEM;
+        //WARN("ixgbe_get_eeprom() invoked, bytes=%u\n", bytes);
 
-	ret_val = hw->eeprom.ops.read_buffer(hw, first_word, eeprom_len,
-					     eeprom_buff);
+        if (eeprom->len == 0)
+                return -EINVAL;
 
-	/* Device's eeprom is always little-endian, word addressable */
-	for (i = 0; i < eeprom_len; i++)
-		le16_to_cpus(&eeprom_buff[i]);
+        magic = hw->vendor_id | (hw->device_id << 16);
+        if (eeprom->magic && eeprom->magic != magic) {
+                nvm = (struct ixgbe_nvm_access *)eeprom;
+                ret_val = ixgbe_nvmupd_command(hw, nvm, bytes);
+                return ret_val;
+        }
 
-	memcpy(bytes, (u8 *)eeprom_buff + (eeprom->offset & 1), eeprom->len);
-	kfree(eeprom_buff);
+        /* normal ethtool get_eeprom support */
+        eeprom->magic = hw->vendor_id | (hw->device_id << 16);
 
-	return ret_val;
-}
+        first_word = eeprom->offset >> 1;
+        last_word = (eeprom->offset + eeprom->len - 1) >> 1;
+        eeprom_len = last_word - first_word + 1;
 
-static int ixgbe_set_eeprom(struct net_device *netdev,
-			    struct ethtool_eeprom *eeprom, u8 *bytes)
-{
-	struct ixgbe_adapter *adapter = netdev_priv(netdev);
-	struct ixgbe_hw *hw = &adapter->hw;
-	u16 *eeprom_buff;
-	void *ptr;
-	int max_len, first_word, last_word, ret_val = 0;
-	u16 i;
+        eeprom_buff = kmalloc(sizeof(u16) * eeprom_len, GFP_KERNEL);
+        if (!eeprom_buff)
+                return -ENOMEM;
 
-	if (eeprom->len == 0)
-		return -EINVAL;
+        ret_val = hw->eeprom.ops.read_buffer(hw, first_word, eeprom_len,
+                                           eeprom_buff);
 
-	if (eeprom->magic != (hw->vendor_id | (hw->device_id << 16)))
-		return -EINVAL;
+        /* Device's eeprom is always little-endian, word addressable */
+        for (i = 0; i < eeprom_len; i++)
+                le16_to_cpus(&eeprom_buff[i]);
 
-	max_len = hw->eeprom.word_size * 2;
+        memcpy(bytes, (u8 *)eeprom_buff + (eeprom->offset & 1), eeprom->len);
+        kfree(eeprom_buff);
 
-	first_word = eeprom->offset >> 1;
-	last_word = (eeprom->offset + eeprom->len - 1) >> 1;
-	eeprom_buff = kmalloc(max_len, GFP_KERNEL);
-	if (!eeprom_buff)
-		return -ENOMEM;
-
-	ptr = eeprom_buff;
-
-	if (eeprom->offset & 1) {
-		/*
-		 * need read/modify/write of first changed EEPROM word
-		 * only the second byte of the word is being modified
-		 */
-		ret_val = hw->eeprom.ops.read(hw, first_word, &eeprom_buff[0]);
-		if (ret_val)
-			goto err;
-
-		ptr++;
-	}
-	if ((eeprom->offset + eeprom->len) & 1) {
-		/*
-		 * need read/modify/write of last changed EEPROM word
-		 * only the first byte of the word is being modified
-		 */
-		ret_val = hw->eeprom.ops.read(hw, last_word,
-					  &eeprom_buff[last_word - first_word]);
-		if (ret_val)
-			goto err;
-	}
-
-	/* Device's eeprom is always little-endian, word addressable */
-	for (i = 0; i < last_word - first_word + 1; i++)
-		le16_to_cpus(&eeprom_buff[i]);
-
-	memcpy(ptr, bytes, eeprom->len);
-
-	for (i = 0; i < last_word - first_word + 1; i++)
-		cpu_to_le16s(&eeprom_buff[i]);
-
-	ret_val = hw->eeprom.ops.write_buffer(hw, first_word,
-					      last_word - first_word + 1,
-					      eeprom_buff);
+        return ret_val;
+}
 
-	/* Update the checksum */
-	if (ret_val == 0)
-		hw->eeprom.ops.update_checksum(hw);
+static int ixgbe_set_eeprom(struct net_device *netdev,
+                            struct ethtool_eeprom *eeprom, u8 *bytes)
+{
+        struct ixgbe_adapter *adapter = netdev_priv(netdev);
+        struct ixgbe_hw *hw = &adapter->hw;
+        int max_len, first_word, last_word, ret_val = 0;
+        struct ixgbe_nvm_access *nvm;
+        u32 magic;
+        u16 *eeprom_buff, i;
+        void *ptr;
+
+        //WARN("ixgbe_set_eeprom() invoked, bytes=%u\n", bytes);
+
+        if (eeprom->len == 0) 
+                return -EINVAL;
+
+        magic = hw->vendor_id | (hw->device_id << 16);
+        if (eeprom->magic && eeprom->magic != magic) {
+                nvm = (struct ixgbe_nvm_access *)eeprom;
+                ret_val = ixgbe_nvmupd_command(hw, nvm, bytes);
+                return ret_val;
+        }
+
+        /* normal ethtool set_eeprom support */
+
+        if (eeprom->magic != (hw->vendor_id | (hw->device_id << 16)))
+                return -EINVAL;
+
+        max_len = hw->eeprom.word_size * 2;
+
+        first_word = eeprom->offset >> 1;
+        last_word = (eeprom->offset + eeprom->len - 1) >> 1;
+        eeprom_buff = kmalloc(max_len, GFP_KERNEL);
+        if (!eeprom_buff)
+                return -ENOMEM;
+
+        ptr = eeprom_buff;
+
+        if (eeprom->offset & 1) {
+                /*
+                 * need read/modify/write of first changed EEPROM word
+                 * only the second byte of the word is being modified
+                 */
+                ret_val = hw->eeprom.ops.read(hw, first_word, &eeprom_buff[0]);
+                if (ret_val)
+                        goto err;
+
+                ptr++;
+        }
+        if (((eeprom->offset + eeprom->len) & 1) && (ret_val == 0)) {
+                /*
+                 * need read/modify/write of last changed EEPROM word
+                 * only the first byte of the word is being modified
+                 */
+                ret_val = hw->eeprom.ops.read(hw, last_word,
+                                          &eeprom_buff[last_word - first_word]);
+                if (ret_val)
+                        goto err;
+        }
+
+        /* Device's eeprom is always little-endian, word addressable */
+        for (i = 0; i < last_word - first_word + 1; i++)
+                le16_to_cpus(&eeprom_buff[i]);
+
+        memcpy(ptr, bytes, eeprom->len);
+
+        for (i = 0; i < last_word - first_word + 1; i++)
+                cpu_to_le16s(&eeprom_buff[i]);
+
+        ret_val = hw->eeprom.ops.write_buffer(hw, first_word,
+                                            last_word - first_word + 1,
+                                            eeprom_buff);
+
+        /* Update the checksum */
+        if (ret_val == 0)
+                hw->eeprom.ops.update_checksum(hw);
 
 err:
-	kfree(eeprom_buff);
-	return ret_val;
+        kfree(eeprom_buff);
+        return ret_val;
 }
 
 static void ixgbe_get_drvinfo(struct net_device *netdev,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 094653e81..ac2405105 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6519,6 +6519,17 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
 	if (nr_cpu_ids > IXGBE_MAX_XDP_QS)
 		static_branch_enable(&ixgbe_xdp_locking_key);
 
+        /* NVM Update features structure initialization */
+        hw->nvmupd_features.major = IXGBE_NVMUPD_FEATURES_API_VER_MAJOR;
+        hw->nvmupd_features.minor = IXGBE_NVMUPD_FEATURES_API_VER_MINOR;
+        hw->nvmupd_features.size = sizeof(hw->nvmupd_features);
+        memset(hw->nvmupd_features.features, 0x0,
+               IXGBE_NVMUPD_FEATURES_API_FEATURES_ARRAY_LEN *
+               sizeof(*hw->nvmupd_features.features));
+
+        hw->nvmupd_features.features[0] =
+                IXGBE_NVMUPD_FEATURE_REGISTER_ACCESS_SUPPORT;
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 346e3d911..5c71e67d2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -129,6 +129,8 @@
 #define IXGBE_GRC_X550EM_x	IXGBE_GRC_8259X
 #define IXGBE_GRC_X550EM_a	0x15F64
 #define IXGBE_GRC(_hw)		IXGBE_BY_MAC((_hw), GRC)
+#define IXGBE_SRAMREL           0x10210
+#define IXGBE_FWRESETCNT        0x15F40
 
 /* General Receive Control */
 #define IXGBE_GRC_MNG  0x00000001 /* Manageability Enable */
@@ -936,6 +938,7 @@ struct ixgbe_nvm_version {
 #define IXGBE_SWSR      0x15F10
 #define IXGBE_HFDR      0x15FE8
 #define IXGBE_FLEX_MNG  0x15800 /* 0x15800 - 0x15EFC */
+#define IXGBE_FLEX_MNG_PTR(_i)  (IXGBE_FLEX_MNG + ((_i) * 4))
 
 #define IXGBE_HICR_EN              0x01  /* Enable bit - RO */
 /* Driver sets this bit when done to put command in RAM */
@@ -3390,6 +3393,38 @@ struct ixgbe_hw_stats {
 	u64 o2bspc;
 };
 
+/* NVM Update commands */
+#define IXGBE_NVMUPD_CMD_REG_READ	0x0000000B
+#define IXGBE_NVMUPD_CMD_REG_WRITE	0x0000000C 
+
+#define IXGBE_R32_Q(h, r) ixgbe_read_reg(h, r)
+#define IXGBE_R8_Q(h, r) readb(READ_ONCE(h->hw_addr) + r)
+
+/* NVM Update features API */
+#define IXGBE_NVMUPD_FEATURES_API_VER_MAJOR             0
+#define IXGBE_NVMUPD_FEATURES_API_VER_MINOR             0
+#define IXGBE_NVMUPD_FEATURES_API_FEATURES_ARRAY_LEN    12
+#define IXGBE_NVMUPD_EXEC_FEATURES                      0xe
+#define IXGBE_NVMUPD_FEATURE_FLAT_NVM_SUPPORT           BIT(0)
+#define IXGBE_NVMUPD_FEATURE_REGISTER_ACCESS_SUPPORT    BIT(1)
+
+#define IXGBE_NVMUPD_MOD_PNT_MASK                       0xFF
+
+struct ixgbe_nvm_access {
+        u32 command;
+        u32 config; 
+        u32 offset;     /* in bytes */
+        u32 data_size;  /* in bytes */
+        u8 data[1];
+};
+
+struct ixgbe_nvm_features {
+        u8 major;
+        u8 minor;
+        u16 size;
+        u8 features[IXGBE_NVMUPD_FEATURES_API_FEATURES_ARRAY_LEN];
+}; 
+
 /* forward declaration */
 struct ixgbe_hw;
 
@@ -3654,6 +3689,8 @@ struct ixgbe_hw {
 	bool				allow_unsupported_sfp;
 	bool				wol_enabled;
 	bool				need_crosstalk_fix;
+       /* NVM Update features */
+        struct ixgbe_nvm_features nvmupd_features;
 };
 
 struct ixgbe_info {
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] ixgbe: Add support for firmware update
  2024-06-09  8:57 [PATCH] ixgbe: Add support for firmware update Richard chien
@ 2024-06-09 11:23 ` Markus Elfring
  2024-06-09 22:33 ` kernel test robot
  2024-06-10 19:36 ` Andrew Lunn
  2 siblings, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2024-06-09 11:23 UTC (permalink / raw)
  To: Richard chien, intel-wired-lan, netdev, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Richard chien, LKML, Jesse Brandeburg, Tony Nguyen

> This patch adds support for firmware update to the in-tree ixgbe driver and it is actually a port
> from the out-of-tree ixgbe driver. In-band firmware update is one of the essential system maintenance
…

Please improve such a change description also according to word wrapping
because of more desirable text line lengths.


…
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -993,114 +993,292 @@ static void ixgbe_get_regs(struct net_device *netdev,
> +static int ixgbe_set_eeprom(struct net_device *netdev,
> +                            struct ethtool_eeprom *eeprom, u8 *bytes)
>  err:
> -	kfree(eeprom_buff);
> -	return ret_val;
> +        kfree(eeprom_buff);
> +        return ret_val;
>  }

Please keep these statements unmodified.

Would you like to reconsider the indentation once more for your change approach?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.10-rc2#n18

Regards,
Markus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ixgbe: Add support for firmware update
  2024-06-09  8:57 [PATCH] ixgbe: Add support for firmware update Richard chien
  2024-06-09 11:23 ` Markus Elfring
@ 2024-06-09 22:33 ` kernel test robot
  2024-06-10 19:36 ` Andrew Lunn
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2024-06-09 22:33 UTC (permalink / raw)
  To: Richard chien, davem, edumazet, kuba, pabeni
  Cc: llvm, oe-kbuild-all, jesse.brandeburg, anthony.l.nguyen,
	intel-wired-lan, netdev, linux-kernel, Richard chien

Hi Richard,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tnguy-net-queue/dev-queue]
[also build test WARNING on linus/master v6.10-rc3 next-20240607]
[cannot apply to tnguy-next-queue/dev-queue]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Richard-chien/ixgbe-Add-support-for-firmware-update/20240609-170239
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue.git dev-queue
patch link:    https://lore.kernel.org/r/20240609085735.6253-1-richard.chien%40hpe.com
patch subject: [PATCH] ixgbe: Add support for firmware update
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240610/202406100635.nORK1Xs0-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240610/202406100635.nORK1Xs0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406100635.nORK1Xs0-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c:1104:9: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
    1104 |         default:
         |         ^
   drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c:1104:9: note: insert 'break;' to avoid fall-through
    1104 |         default:
         |         ^
         |         break; 
   1 warning generated.


vim +1104 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c

  1005	
  1006	static int ixgbe_nvmupd_validate_offset(struct ixgbe_adapter *adapter,
  1007	                                        u32 offset)
  1008	{
  1009	        struct net_device *netdev = adapter->netdev;
  1010	
  1011	        switch (offset) {
  1012	        case IXGBE_STATUS:
  1013	        case IXGBE_ESDP:
  1014	        case IXGBE_MSCA:
  1015	        case IXGBE_MSRWD:
  1016	        case IXGBE_EEC_8259X:
  1017	        case IXGBE_FLA_8259X:
  1018	        case IXGBE_FLOP:
  1019	        case IXGBE_SWSM_8259X:
  1020	        case IXGBE_FWSM_8259X:
  1021	        case IXGBE_FACTPS_8259X:
  1022	        case IXGBE_GSSR:
  1023	        case IXGBE_HICR:
  1024	        case IXGBE_FWSTS:
  1025	                return 0;
  1026	        default:
  1027	                if ((offset >= IXGBE_MAVTV(0) && offset <= IXGBE_MAVTV(7)) ||
  1028	                    (offset >= IXGBE_RAL(0) && offset <= IXGBE_RAH(15)))
  1029	                        return 0;
  1030	        }
  1031	
  1032	        switch (adapter->hw.mac.type) {
  1033	        case ixgbe_mac_82599EB:
  1034	                switch (offset) {
  1035	                case IXGBE_AUTOC:
  1036	                case IXGBE_EERD:
  1037	                case IXGBE_BARCTRL:
  1038	                        return 0;
  1039	                default:
  1040	                        if (offset >= 0x00020000 &&
  1041	                            offset <= ixgbe_get_eeprom_len(netdev))
  1042	                                return 0;
  1043	                }
  1044	                break;
  1045	        case ixgbe_mac_X540:
  1046	                switch (offset) {
  1047	                case IXGBE_EERD:
  1048	                case IXGBE_EEWR:
  1049	                case IXGBE_SRAMREL:
  1050	                case IXGBE_BARCTRL:
  1051	                        return 0;
  1052	                default:
  1053	                        if ((offset >= 0x00020000 &&
  1054	                             offset <= ixgbe_get_eeprom_len(netdev)))
  1055	                                return 0;
  1056	                }
  1057	                break;
  1058	        case ixgbe_mac_X550:
  1059	                switch (offset) {
  1060	                case IXGBE_EEWR:
  1061	                case IXGBE_SRAMREL:
  1062	                case IXGBE_PHYCTL_82599:
  1063	                case IXGBE_FWRESETCNT:
  1064	                        return 0;
  1065	                default:
  1066	                        if (offset >= IXGBE_FLEX_MNG_PTR(0) &&
  1067	                            offset <= IXGBE_FLEX_MNG_PTR(447))
  1068	                                return 0;
  1069	                }
  1070	                break;
  1071	        case ixgbe_mac_X550EM_x:
  1072	                switch (offset) {
  1073	                case IXGBE_PHYCTL_82599:
  1074	                case IXGBE_NW_MNG_IF_SEL:
  1075	                case IXGBE_FWRESETCNT:
  1076	                case IXGBE_I2CCTL_X550:
  1077	                        return 0;
  1078	                default:
  1079	                        if ((offset >= IXGBE_FLEX_MNG_PTR(0) &&
  1080	                             offset <= IXGBE_FLEX_MNG_PTR(447)) ||
  1081	                            (offset >= IXGBE_FUSES0_GROUP(0) &&
  1082	                             offset <= IXGBE_FUSES0_GROUP(7)))
  1083	                                return 0;
  1084	                }
  1085	                break;
  1086	        case ixgbe_mac_x550em_a:
  1087	                switch (offset) {
  1088	                case IXGBE_PHYCTL_82599:
  1089	                case IXGBE_NW_MNG_IF_SEL:
  1090	                case IXGBE_FWRESETCNT:
  1091	                case IXGBE_I2CCTL_X550:
  1092	                case IXGBE_FLA_X550EM_a:
  1093	                case IXGBE_SWSM_X550EM_a:
  1094	                case IXGBE_FWSM_X550EM_a:
  1095	                case IXGBE_SWFW_SYNC_X550EM_a:
  1096	                case IXGBE_FACTPS_X550EM_a:
  1097	                case IXGBE_EEC_X550EM_a:
  1098	                        return 0;
  1099	                default:
  1100	                        if (offset >= IXGBE_FLEX_MNG_PTR(0) &&
  1101	                            offset <= IXGBE_FLEX_MNG_PTR(447))
  1102	                                return 0;
  1103	                }
> 1104	        default:
  1105	                break;
  1106	        }
  1107	
  1108	        return -ENOTTY;
  1109	}
  1110	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ixgbe: Add support for firmware update
  2024-06-09  8:57 [PATCH] ixgbe: Add support for firmware update Richard chien
  2024-06-09 11:23 ` Markus Elfring
  2024-06-09 22:33 ` kernel test robot
@ 2024-06-10 19:36 ` Andrew Lunn
  2024-06-11  7:12   ` Chien, Richard (Options Engineering)
  2 siblings, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2024-06-10 19:36 UTC (permalink / raw)
  To: Richard chien
  Cc: davem, edumazet, kuba, pabeni, jesse.brandeburg, anthony.l.nguyen,
	intel-wired-lan, netdev, linux-kernel, Richard chien

On Sun, Jun 09, 2024 at 04:57:35PM +0800, Richard chien wrote:
> This patch adds support for firmware update to the in-tree ixgbe driver and it is actually a port
> from the out-of-tree ixgbe driver. In-band firmware update is one of the essential system maintenance
> tasks. To simplify this task, the Intel online firmware update utility provides a common interface
> that works across different Intel NICs running the igb/ixgbe/i40e drivers. Unfortunately, the in-tree
> igb and ixgbe drivers are unable to support this firmware update utility, causing problems for
> enterprise users who do not or cannot use out-of-distro drivers due to security and various other
> reasons (e.g. commercial Linux distros do not provide technical support for out-of-distro drivers).
> As a result, getting this feature into the in-tree ixgbe driver is highly desirable.
> 
> Signed-off-by: Richard chien <richard.chien@hpe.com>

How about you work on one driver at a time, to learn about the
processes for submitting to the Linux kernel.

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#netdev-faq

https://docs.kernel.org/process/submitting-patches.html

https://www.kernel.org/doc/html/latest/process/coding-style.html

I would also think about why Intel has not submitted this code before?
Maybe because it does things the wrong way? Please look at how other
Ethernet drivers support firmware. Is it the same? It might be you
need to throw away this code and reimplement it to mainline standards,
maybe using devlink flash, or ethtool -f.

One additional question. Is the firmware part of linux-firmware? Given
this is Intel, i expect the firmware is distributeable, but have they
distributed it?

	Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] ixgbe: Add support for firmware update
  2024-06-10 19:36 ` Andrew Lunn
@ 2024-06-11  7:12   ` Chien, Richard (Options Engineering)
  0 siblings, 0 replies; 5+ messages in thread
From: Chien, Richard (Options Engineering) @ 2024-06-11  7:12 UTC (permalink / raw)
  To: Andrew Lunn, Richard chien
  Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, jesse.brandeburg@intel.com,
	anthony.l.nguyen@intel.com, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org

> I would also think about why Intel has not submitted this code before?
> Maybe because it does things the wrong way? Please look at how other
> Ethernet drivers support firmware. Is it the same? It might be you need to
> throw away this code and reimplement it to mainline standards, maybe using
> devlink flash, or ethtool -f.

See Jacob's reply for details.
 
> One additional question. Is the firmware part of linux-firmware? Given this is
> Intel, i expect the firmware is distributeable, but have they distributed it?

It is the Intel 10G NIC firmware embedded into HPE firmware update packages and redistributed to the end user.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-06-11  7:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-09  8:57 [PATCH] ixgbe: Add support for firmware update Richard chien
2024-06-09 11:23 ` Markus Elfring
2024-06-09 22:33 ` kernel test robot
2024-06-10 19:36 ` Andrew Lunn
2024-06-11  7:12   ` Chien, Richard (Options Engineering)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).