| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZiZzdAX-qI-7wCMC@nanopsycho>
Date: Mon, 22 Apr 2024 16:25:56 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Piotr Kwapulinski <piotr.kwapulinski@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
horms@...nel.org, Carolyn Wyborny <carolyn.wyborny@...el.com>,
Jedrzej Jagielski <jedrzej.jagielski@...el.com>,
Jan Glaza <jan.glaza@...el.com>
Subject: Re: [PATCH iwl-next v4 5/5] ixgbe: Enable link management in E610
device
Mon, Apr 22, 2024 at 03:06:11PM CEST, piotr.kwapulinski@...el.com wrote:
[...]
>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>index 559b443..ea6df1e 100644
>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>@@ -1,5 +1,5 @@
> /* SPDX-License-Identifier: GPL-2.0 */
>-/* Copyright(c) 1999 - 2018 Intel Corporation. */
>+/* Copyright(c) 1999 - 2024 Intel Corporation. */
>
> #ifndef _IXGBE_H_
> #define _IXGBE_H_
>@@ -20,6 +20,7 @@
> #include "ixgbe_type.h"
> #include "ixgbe_common.h"
> #include "ixgbe_dcb.h"
>+#include "ixgbe_e610.h"
> #if IS_ENABLED(CONFIG_FCOE)
> #define IXGBE_FCOE
> #include "ixgbe_fcoe.h"
>@@ -28,7 +29,6 @@
> #include <linux/dca.h>
> #endif
> #include "ixgbe_ipsec.h"
>-
Leftover hunk?
> #include <net/xdp.h>
>
> /* common prefix used by pr_<> macros */
[...]
>+static const struct ixgbe_mac_operations mac_ops_e610 = {
>+ .init_hw = ixgbe_init_hw_generic,
>+ .start_hw = ixgbe_start_hw_X540,
>+ .clear_hw_cntrs = ixgbe_clear_hw_cntrs_generic,
>+ .enable_rx_dma = ixgbe_enable_rx_dma_generic,
>+ .get_mac_addr = ixgbe_get_mac_addr_generic,
>+ .get_device_caps = ixgbe_get_device_caps_generic,
>+ .stop_adapter = ixgbe_stop_adapter_generic,
>+ .set_lan_id = ixgbe_set_lan_id_multi_port_pcie,
>+ .read_analog_reg8 = NULL,
Pointless initialization, it's null already. You have many cases of
this below.
>+ .write_analog_reg8 = NULL,
>+ .set_rxpba = ixgbe_set_rxpba_generic,
>+ .check_link = ixgbe_check_link_e610,
>+ .blink_led_start = ixgbe_blink_led_start_X540,
>+ .blink_led_stop = ixgbe_blink_led_stop_X540,
>+ .set_rar = ixgbe_set_rar_generic,
>+ .clear_rar = ixgbe_clear_rar_generic,
>+ .set_vmdq = ixgbe_set_vmdq_generic,
>+ .set_vmdq_san_mac = ixgbe_set_vmdq_san_mac_generic,
>+ .clear_vmdq = ixgbe_clear_vmdq_generic,
>+ .init_rx_addrs = ixgbe_init_rx_addrs_generic,
>+ .update_mc_addr_list = ixgbe_update_mc_addr_list_generic,
>+ .enable_mc = ixgbe_enable_mc_generic,
>+ .disable_mc = ixgbe_disable_mc_generic,
>+ .clear_vfta = ixgbe_clear_vfta_generic,
>+ .set_vfta = ixgbe_set_vfta_generic,
>+ .fc_enable = ixgbe_fc_enable_generic,
>+ .set_fw_drv_ver = ixgbe_set_fw_drv_ver_x550,
>+ .init_uta_tables = ixgbe_init_uta_tables_generic,
>+ .set_mac_anti_spoofing = ixgbe_set_mac_anti_spoofing,
>+ .set_vlan_anti_spoofing = ixgbe_set_vlan_anti_spoofing,
>+ .set_source_address_pruning =
>+ ixgbe_set_source_address_pruning_x550,
>+ .set_ethertype_anti_spoofing =
>+ ixgbe_set_ethertype_anti_spoofing_x550,
>+ .disable_rx_buff = ixgbe_disable_rx_buff_generic,
>+ .enable_rx_buff = ixgbe_enable_rx_buff_generic,
>+ .get_thermal_sensor_data = NULL,
>+ .init_thermal_sensor_thresh = NULL,
>+ .fw_recovery_mode = NULL,
>+ .enable_rx = ixgbe_enable_rx_generic,
>+ .disable_rx = ixgbe_disable_rx_e610,
>+ .led_on = ixgbe_led_on_generic,
>+ .led_off = ixgbe_led_off_generic,
>+ .init_led_link_act = ixgbe_init_led_link_act_generic,
>+ .reset_hw = ixgbe_reset_hw_e610,
>+ .get_media_type = ixgbe_get_media_type_e610,
>+ .get_san_mac_addr = NULL,
>+ .get_wwn_prefix = NULL,
>+ .setup_link = ixgbe_setup_link_e610,
>+ .get_link_capabilities = ixgbe_get_link_capabilities_e610,
>+ .get_bus_info = ixgbe_get_bus_info_generic,
>+ .setup_sfp = NULL,
>+ .acquire_swfw_sync = ixgbe_acquire_swfw_sync_X540,
>+ .release_swfw_sync = ixgbe_release_swfw_sync_X540,
>+ .init_swfw_sync = ixgbe_init_swfw_sync_X540,
>+ .prot_autoc_read = prot_autoc_read_generic,
>+ .prot_autoc_write = prot_autoc_write_generic,
>+ .setup_fc = ixgbe_setup_fc_e610,
>+ .fc_autoneg = ixgbe_fc_autoneg_e610,
>+};
>+
>+static const struct ixgbe_phy_operations phy_ops_e610 = {
>+ .init = ixgbe_init_phy_ops_e610,
>+ .identify = ixgbe_identify_phy_e610,
>+ .read_reg = NULL,
>+ .write_reg = NULL,
>+ .read_reg_mdi = NULL,
>+ .write_reg_mdi = NULL,
>+ .identify_sfp = ixgbe_identify_module_e610,
>+ .reset = NULL,
>+ .setup_link_speed = ixgbe_setup_phy_link_speed_generic,
>+ .read_i2c_byte = NULL,
>+ .write_i2c_byte = NULL,
>+ .read_i2c_sff8472 = NULL,
>+ .read_i2c_eeprom = NULL,
>+ .write_i2c_eeprom = NULL,
>+ .setup_link = ixgbe_setup_phy_link_e610,
>+ .set_phy_power = NULL,
>+ .check_overtemp = NULL,
>+ .enter_lplu = ixgbe_enter_lplu_e610,
>+ .handle_lasi = NULL,
>+ .read_i2c_byte_unlocked = NULL,
>+};
>+
>+static const struct ixgbe_eeprom_operations eeprom_ops_e610 = {
>+ .init_params = NULL,
>+ .read = ixgbe_read_ee_aci_e610,
>+ .read_buffer = NULL,
>+ .write = NULL,
>+ .write_buffer = NULL,
>+ .validate_checksum = ixgbe_validate_eeprom_checksum_e610,
>+ .update_checksum = NULL,
>+ .calc_checksum = NULL,
>+};
>+
[...]
>+/**
>+ * ixgbe_process_link_status_event - process the link event
>+ * @adapter: pointer to adapter structure
>+ * @link_up: true if the physical link is up and false if it is down
>+ * @link_speed: current link speed received from the link event
>+ *
>+ * Return: 0 on success and negative on failure.
>+ */
>+static int
>+ixgbe_process_link_status_event(struct ixgbe_adapter *adapter, bool link_up,
>+ u16 link_speed)
>+{
>+ struct ixgbe_hw *hw = &adapter->hw;
>+ int status;
>+
>+ /* update the link info structures and re-enable link events,
>+ * don't bail on failure due to other book keeping needed
Why don't you start the sentence with capital letter and end with "."?
>+ */
>+ status = ixgbe_update_link_info(hw);
>+ if (status)
>+ e_dev_err("Failed to update link status, err %d aq_err %d\n",
>+ status, hw->aci.last_status);
>+
>+ ixgbe_check_link_cfg_err(adapter, hw->link.link_info.link_cfg_err);
>+
>+ /* Check if the link state is up after updating link info, and treat
>+ * this event as an UP event since the link is actually UP now.
>+ */
>+ if (hw->link.link_info.link_info & IXGBE_ACI_LINK_UP)
>+ link_up = true;
>+
>+ /* turn off PHY if media was removed */
>+ if (!(adapter->flags2 & IXGBE_FLAG2_NO_MEDIA) &&
>+ !(hw->link.link_info.link_info & IXGBE_ACI_MEDIA_AVAILABLE)) {
>+ (adapter->flags2 |= IXGBE_FLAG2_NO_MEDIA);
>+ if (ixgbe_aci_set_link_restart_an(hw, false))
>+ e_dev_err("can't set link to OFF\n");
>+ }
[...]
Powered by blists - more mailing lists