netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iwl-next 0/3] intel: misc improvements
@ 2024-02-22 14:50 Maciej Fijalkowski
  2024-02-22 14:50 ` [PATCH iwl-next 1/3] ice: do not disable Tx queues twice in ice_down() Maciej Fijalkowski
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Maciej Fijalkowski @ 2024-02-22 14:50 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson, Maciej Fijalkowski

Hi,

here are not related improvements to ice and ixgbe. Spotted while
working on other issues. First one takes care redundant Tx disabling on
ifdown. Second one is about rather obvious getting rid of devm_ usage
and last one is plain refactor of stats update.

Thanks!

Maciej Fijalkowski (3):
  ice: do not disable Tx queues twice in ice_down()
  ice: avoid unnecessary devm_ usage
  ixgbe: pull out stats update to common routines

 drivers/net/ethernet/intel/ice/ice_common.c   | 23 ++++----
 drivers/net/ethernet/intel/ice/ice_ethtool.c  |  4 +-
 drivers/net/ethernet/intel/ice/ice_lib.c      | 55 -------------------
 drivers/net/ethernet/intel/ice/ice_lib.h      |  2 -
 drivers/net/ethernet/intel/ice/ice_main.c     | 44 +++++++++++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 54 ++++++++++++++----
 .../ethernet/intel/ixgbe/ixgbe_txrx_common.h  |  7 +++
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c  | 17 ++----
 8 files changed, 109 insertions(+), 97 deletions(-)

-- 
2.34.1


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

* [PATCH iwl-next 1/3] ice: do not disable Tx queues twice in ice_down()
  2024-02-22 14:50 [PATCH iwl-next 0/3] intel: misc improvements Maciej Fijalkowski
@ 2024-02-22 14:50 ` Maciej Fijalkowski
  2024-02-22 14:50 ` [PATCH iwl-next 2/3] ice: avoid unnecessary devm_ usage Maciej Fijalkowski
  2024-02-22 14:50 ` [PATCH iwl-next 3/3] ixgbe: pull out stats update to common routines Maciej Fijalkowski
  2 siblings, 0 replies; 7+ messages in thread
From: Maciej Fijalkowski @ 2024-02-22 14:50 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson, Maciej Fijalkowski

ice_down() clears QINT_TQCTL_CAUSE_ENA_M bit twice, which is not
necessary. First clearing happens in ice_vsi_dis_irq() and second in
ice_vsi_stop_tx_ring() - remove the first one.

While at it, make ice_vsi_dis_irq() static as ice_down() is the only
current caller of it.

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_lib.c  | 55 -----------------------
 drivers/net/ethernet/intel/ice/ice_lib.h  |  2 -
 drivers/net/ethernet/intel/ice/ice_main.c | 44 ++++++++++++++++++
 3 files changed, 44 insertions(+), 57 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 9be724291ef8..9323bc1d4bdc 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -2848,61 +2848,6 @@ void ice_dis_vsi(struct ice_vsi *vsi, bool locked)
 	}
 }
 
-/**
- * ice_vsi_dis_irq - Mask off queue interrupt generation on the VSI
- * @vsi: the VSI being un-configured
- */
-void ice_vsi_dis_irq(struct ice_vsi *vsi)
-{
-	struct ice_pf *pf = vsi->back;
-	struct ice_hw *hw = &pf->hw;
-	u32 val;
-	int i;
-
-	/* disable interrupt causation from each queue */
-	if (vsi->tx_rings) {
-		ice_for_each_txq(vsi, i) {
-			if (vsi->tx_rings[i]) {
-				u16 reg;
-
-				reg = vsi->tx_rings[i]->reg_idx;
-				val = rd32(hw, QINT_TQCTL(reg));
-				val &= ~QINT_TQCTL_CAUSE_ENA_M;
-				wr32(hw, QINT_TQCTL(reg), val);
-			}
-		}
-	}
-
-	if (vsi->rx_rings) {
-		ice_for_each_rxq(vsi, i) {
-			if (vsi->rx_rings[i]) {
-				u16 reg;
-
-				reg = vsi->rx_rings[i]->reg_idx;
-				val = rd32(hw, QINT_RQCTL(reg));
-				val &= ~QINT_RQCTL_CAUSE_ENA_M;
-				wr32(hw, QINT_RQCTL(reg), val);
-			}
-		}
-	}
-
-	/* disable each interrupt */
-	ice_for_each_q_vector(vsi, i) {
-		if (!vsi->q_vectors[i])
-			continue;
-		wr32(hw, GLINT_DYN_CTL(vsi->q_vectors[i]->reg_idx), 0);
-	}
-
-	ice_flush(hw);
-
-	/* don't call synchronize_irq() for VF's from the host */
-	if (vsi->type == ICE_VSI_VF)
-		return;
-
-	ice_for_each_q_vector(vsi, i)
-		synchronize_irq(vsi->q_vectors[i]->irq.virq);
-}
-
 /**
  * ice_queue_set_napi - Set the napi instance for the queue
  * @dev: device to which NAPI and queue belong
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h b/drivers/net/ethernet/intel/ice/ice_lib.h
index 71bd27244941..c394fe9abba4 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_lib.h
@@ -114,8 +114,6 @@ void
 ice_write_qrxflxp_cntxt(struct ice_hw *hw, u16 pf_q, u32 rxdid, u32 prio,
 			bool ena_ts);
 
-void ice_vsi_dis_irq(struct ice_vsi *vsi);
-
 void ice_vsi_free_irq(struct ice_vsi *vsi);
 
 void ice_vsi_free_rx_rings(struct ice_vsi *vsi);
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index dd4a9bc0dfdc..2358977324fd 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -7059,6 +7059,50 @@ static void ice_napi_disable_all(struct ice_vsi *vsi)
 	}
 }
 
+/**
+ * ice_vsi_dis_irq - Mask off queue interrupt generation on the VSI
+ * @vsi: the VSI being un-configured
+ */
+static void ice_vsi_dis_irq(struct ice_vsi *vsi)
+{
+	struct ice_pf *pf = vsi->back;
+	struct ice_hw *hw = &pf->hw;
+	u32 val;
+	int i;
+
+	/* disable interrupt causation from each Rx queue; Tx queues are
+	 * handled in ice_vsi_stop_tx_ring()
+	 */
+	if (vsi->rx_rings) {
+		ice_for_each_rxq(vsi, i) {
+			if (vsi->rx_rings[i]) {
+				u16 reg;
+
+				reg = vsi->rx_rings[i]->reg_idx;
+				val = rd32(hw, QINT_RQCTL(reg));
+				val &= ~QINT_RQCTL_CAUSE_ENA_M;
+				wr32(hw, QINT_RQCTL(reg), val);
+			}
+		}
+	}
+
+	/* disable each interrupt */
+	ice_for_each_q_vector(vsi, i) {
+		if (!vsi->q_vectors[i])
+			continue;
+		wr32(hw, GLINT_DYN_CTL(vsi->q_vectors[i]->reg_idx), 0);
+	}
+
+	ice_flush(hw);
+
+	/* don't call synchronize_irq() for VF's from the host */
+	if (vsi->type == ICE_VSI_VF)
+		return;
+
+	ice_for_each_q_vector(vsi, i)
+		synchronize_irq(vsi->q_vectors[i]->irq.virq);
+}
+
 /**
  * ice_down - Shutdown the connection
  * @vsi: The VSI being stopped
-- 
2.34.1


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

* [PATCH iwl-next 2/3] ice: avoid unnecessary devm_ usage
  2024-02-22 14:50 [PATCH iwl-next 0/3] intel: misc improvements Maciej Fijalkowski
  2024-02-22 14:50 ` [PATCH iwl-next 1/3] ice: do not disable Tx queues twice in ice_down() Maciej Fijalkowski
@ 2024-02-22 14:50 ` Maciej Fijalkowski
  2024-02-23  7:45   ` Przemek Kitszel
  2024-02-23  9:35   ` kernel test robot
  2024-02-22 14:50 ` [PATCH iwl-next 3/3] ixgbe: pull out stats update to common routines Maciej Fijalkowski
  2 siblings, 2 replies; 7+ messages in thread
From: Maciej Fijalkowski @ 2024-02-22 14:50 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson, Maciej Fijalkowski

1. pcaps are free'd right after AQ routines are done, no need for
   devm_'s
2. a test frame for loopback test in ethtool -t is destroyed at the end
   of the test so we don't need devm_ here either.

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_common.c  | 23 +++++++++-----------
 drivers/net/ethernet/intel/ice/ice_ethtool.c |  4 ++--
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 10c32cd80fff..6de93e12ead3 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -1045,7 +1045,7 @@ int ice_init_hw(struct ice_hw *hw)
 	if (status)
 		goto err_unroll_sched;
 
-	pcaps = devm_kzalloc(ice_hw_to_dev(hw), sizeof(*pcaps), GFP_KERNEL);
+	pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL);
 	if (!pcaps) {
 		status = -ENOMEM;
 		goto err_unroll_sched;
@@ -1055,7 +1055,7 @@ int ice_init_hw(struct ice_hw *hw)
 	status = ice_aq_get_phy_caps(hw->port_info, false,
 				     ICE_AQC_REPORT_TOPO_CAP_MEDIA, pcaps,
 				     NULL);
-	devm_kfree(ice_hw_to_dev(hw), pcaps);
+	kfree(pcaps);
 	if (status)
 		dev_warn(ice_hw_to_dev(hw), "Get PHY capabilities failed status = %d, continuing anyway\n",
 			 status);
@@ -1082,18 +1082,16 @@ int ice_init_hw(struct ice_hw *hw)
 
 	/* Get MAC information */
 	/* A single port can report up to two (LAN and WoL) addresses */
-	mac_buf = devm_kcalloc(ice_hw_to_dev(hw), 2,
-			       sizeof(struct ice_aqc_manage_mac_read_resp),
-			       GFP_KERNEL);
-	mac_buf_len = 2 * sizeof(struct ice_aqc_manage_mac_read_resp);
-
+	mac_buf = kcalloc(2, sizeof(struct ice_aqc_manage_mac_read_resp),
+			  GFP_KERNEL);
 	if (!mac_buf) {
 		status = -ENOMEM;
 		goto err_unroll_fltr_mgmt_struct;
 	}
 
+	mac_buf_len = 2 * sizeof(struct ice_aqc_manage_mac_read_resp);
 	status = ice_aq_manage_mac_read(hw, mac_buf, mac_buf_len, NULL);
-	devm_kfree(ice_hw_to_dev(hw), mac_buf);
+	kfree(mac_buf);
 
 	if (status)
 		goto err_unroll_fltr_mgmt_struct;
@@ -3244,15 +3242,14 @@ int ice_update_link_info(struct ice_port_info *pi)
 		struct ice_hw *hw;
 
 		hw = pi->hw;
-		pcaps = devm_kzalloc(ice_hw_to_dev(hw), sizeof(*pcaps),
-				     GFP_KERNEL);
+		pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL);
 		if (!pcaps)
 			return -ENOMEM;
 
 		status = ice_aq_get_phy_caps(pi, false, ICE_AQC_REPORT_TOPO_CAP_MEDIA,
 					     pcaps, NULL);
 
-		devm_kfree(ice_hw_to_dev(hw), pcaps);
+		kfree(pcaps);
 	}
 
 	return status;
@@ -3404,7 +3401,7 @@ ice_set_fc(struct ice_port_info *pi, u8 *aq_failures, bool ena_auto_link_update)
 	*aq_failures = 0;
 	hw = pi->hw;
 
-	pcaps = devm_kzalloc(ice_hw_to_dev(hw), sizeof(*pcaps), GFP_KERNEL);
+	pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL);
 	if (!pcaps)
 		return -ENOMEM;
 
@@ -3456,7 +3453,7 @@ ice_set_fc(struct ice_port_info *pi, u8 *aq_failures, bool ena_auto_link_update)
 	}
 
 out:
-	devm_kfree(ice_hw_to_dev(hw), pcaps);
+	kfree(pcaps);
 	return status;
 }
 
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index a19b06f18e40..cec3d796546e 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -801,7 +801,7 @@ static int ice_lbtest_create_frame(struct ice_pf *pf, u8 **ret_data, u16 size)
 	if (!pf)
 		return -EINVAL;
 
-	data = devm_kzalloc(ice_pf_to_dev(pf), size, GFP_KERNEL);
+	data = kzalloc(size, GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
@@ -1004,7 +1004,7 @@ static u64 ice_loopback_test(struct net_device *netdev)
 		ret = 10;
 
 lbtest_free_frame:
-	devm_kfree(dev, tx_frame);
+	kfree(tx_frame);
 remove_mac_filters:
 	if (ice_fltr_remove_mac(test_vsi, broadcast, ICE_FWD_TO_VSI))
 		netdev_err(netdev, "Could not remove MAC filter for the test VSI\n");
-- 
2.34.1


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

* [PATCH iwl-next 3/3] ixgbe: pull out stats update to common routines
  2024-02-22 14:50 [PATCH iwl-next 0/3] intel: misc improvements Maciej Fijalkowski
  2024-02-22 14:50 ` [PATCH iwl-next 1/3] ice: do not disable Tx queues twice in ice_down() Maciej Fijalkowski
  2024-02-22 14:50 ` [PATCH iwl-next 2/3] ice: avoid unnecessary devm_ usage Maciej Fijalkowski
@ 2024-02-22 14:50 ` Maciej Fijalkowski
  2 siblings, 0 replies; 7+ messages in thread
From: Maciej Fijalkowski @ 2024-02-22 14:50 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson, Maciej Fijalkowski

Introduce ixgbe_update_{r,t}x_ring_stats() that will be used by both
standard and ZC datapath.

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 54 ++++++++++++++-----
 .../ethernet/intel/ixgbe/ixgbe_txrx_common.h  |  7 +++
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c  | 17 ++----
 3 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bd541527c8c7..ee0321db61f8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1105,6 +1105,44 @@ static int ixgbe_tx_maxrate(struct net_device *netdev,
 	return 0;
 }
 
+/**
+ * ixgbe_update_tx_ring_stats - Update Tx ring specific counters
+ * @tx_ring: ring to update
+ * @q_vector: queue vector ring belongs to
+ * @pkts: number of processed packets
+ * @bytes: number of processed bytes
+ */
+void ixgbe_update_tx_ring_stats(struct ixgbe_ring *tx_ring,
+				struct ixgbe_q_vector *q_vector, u64 pkts,
+				u64 bytes)
+{
+	u64_stats_update_begin(&tx_ring->syncp);
+	tx_ring->stats.bytes += bytes;
+	tx_ring->stats.packets += pkts;
+	u64_stats_update_end(&tx_ring->syncp);
+	q_vector->tx.total_bytes += bytes;
+	q_vector->tx.total_packets += pkts;
+}
+
+/**
+ * ixgbe_update_rx_ring_stats - Update Rx ring specific counters
+ * @rx_ring: ring to update
+ * @q_vector: queue vector ring belongs to
+ * @pkts: number of processed packets
+ * @bytes: number of processed bytes
+ */
+void ixgbe_update_rx_ring_stats(struct ixgbe_ring *rx_ring,
+				struct ixgbe_q_vector *q_vector, u64 pkts,
+				u64 bytes)
+{
+	u64_stats_update_begin(&rx_ring->syncp);
+	rx_ring->stats.bytes += bytes;
+	rx_ring->stats.packets += pkts;
+	u64_stats_update_end(&rx_ring->syncp);
+	q_vector->rx.total_bytes += bytes;
+	q_vector->rx.total_packets += pkts;
+}
+
 /**
  * ixgbe_clean_tx_irq - Reclaim resources after transmit completes
  * @q_vector: structure containing interrupt and ring information
@@ -1207,12 +1245,8 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
 
 	i += tx_ring->count;
 	tx_ring->next_to_clean = i;
-	u64_stats_update_begin(&tx_ring->syncp);
-	tx_ring->stats.bytes += total_bytes;
-	tx_ring->stats.packets += total_packets;
-	u64_stats_update_end(&tx_ring->syncp);
-	q_vector->tx.total_bytes += total_bytes;
-	q_vector->tx.total_packets += total_packets;
+	ixgbe_update_tx_ring_stats(tx_ring, q_vector, total_packets,
+				   total_bytes);
 	adapter->tx_ipsec += total_ipsec;
 
 	if (check_for_tx_hang(tx_ring) && ixgbe_check_tx_hang(tx_ring)) {
@@ -2429,12 +2463,8 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 		ixgbe_xdp_ring_update_tail_locked(ring);
 	}
 
-	u64_stats_update_begin(&rx_ring->syncp);
-	rx_ring->stats.packets += total_rx_packets;
-	rx_ring->stats.bytes += total_rx_bytes;
-	u64_stats_update_end(&rx_ring->syncp);
-	q_vector->rx.total_packets += total_rx_packets;
-	q_vector->rx.total_bytes += total_rx_bytes;
+	ixgbe_update_rx_ring_stats(rx_ring, q_vector, total_rx_packets,
+				   total_rx_bytes);
 
 	return total_rx_packets;
 }
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
index f1f69ce67420..78deea5ec536 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
@@ -46,4 +46,11 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
 int ixgbe_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags);
 void ixgbe_xsk_clean_tx_ring(struct ixgbe_ring *tx_ring);
 
+void ixgbe_update_tx_ring_stats(struct ixgbe_ring *tx_ring,
+				struct ixgbe_q_vector *q_vector, u64 pkts,
+				u64 bytes);
+void ixgbe_update_rx_ring_stats(struct ixgbe_ring *rx_ring,
+				struct ixgbe_q_vector *q_vector, u64 pkts,
+				u64 bytes);
+
 #endif /* #define _IXGBE_TXRX_COMMON_H_ */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 59798bc33298..d34d715c59eb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -359,12 +359,8 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
 		ixgbe_xdp_ring_update_tail_locked(ring);
 	}
 
-	u64_stats_update_begin(&rx_ring->syncp);
-	rx_ring->stats.packets += total_rx_packets;
-	rx_ring->stats.bytes += total_rx_bytes;
-	u64_stats_update_end(&rx_ring->syncp);
-	q_vector->rx.total_packets += total_rx_packets;
-	q_vector->rx.total_bytes += total_rx_bytes;
+	ixgbe_update_rx_ring_stats(rx_ring, q_vector, total_rx_packets,
+				   total_rx_bytes);
 
 	if (xsk_uses_need_wakeup(rx_ring->xsk_pool)) {
 		if (failure || rx_ring->next_to_clean == rx_ring->next_to_use)
@@ -499,13 +495,8 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
 	}
 
 	tx_ring->next_to_clean = ntc;
-
-	u64_stats_update_begin(&tx_ring->syncp);
-	tx_ring->stats.bytes += total_bytes;
-	tx_ring->stats.packets += total_packets;
-	u64_stats_update_end(&tx_ring->syncp);
-	q_vector->tx.total_bytes += total_bytes;
-	q_vector->tx.total_packets += total_packets;
+	ixgbe_update_tx_ring_stats(tx_ring, q_vector, total_packets,
+				   total_bytes);
 
 	if (xsk_frames)
 		xsk_tx_completed(pool, xsk_frames);
-- 
2.34.1


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

* Re: [PATCH iwl-next 2/3] ice: avoid unnecessary devm_ usage
  2024-02-22 14:50 ` [PATCH iwl-next 2/3] ice: avoid unnecessary devm_ usage Maciej Fijalkowski
@ 2024-02-23  7:45   ` Przemek Kitszel
  2024-02-23 15:03     ` Maciej Fijalkowski
  2024-02-23  9:35   ` kernel test robot
  1 sibling, 1 reply; 7+ messages in thread
From: Przemek Kitszel @ 2024-02-23  7:45 UTC (permalink / raw)
  To: Maciej Fijalkowski, intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson

On 2/22/24 15:50, Maciej Fijalkowski wrote:
> 1. pcaps are free'd right after AQ routines are done, no need for
>     devm_'s
> 2. a test frame for loopback test in ethtool -t is destroyed at the end
>     of the test so we don't need devm_ here either.
> 
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_common.c  | 23 +++++++++-----------
>   drivers/net/ethernet/intel/ice/ice_ethtool.c |  4 ++--
>   2 files changed, 12 insertions(+), 15 deletions(-)
> 

nice, thank you!

BTW, we are committed to using Scope Based Resource Management [1] even
in the OOT driver, so you could consider that for future IWL submissions
:)

[1] https://lwn.net/Articles/934679/


I'm also happy with this as-is too, so:
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

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

* Re: [PATCH iwl-next 2/3] ice: avoid unnecessary devm_ usage
  2024-02-22 14:50 ` [PATCH iwl-next 2/3] ice: avoid unnecessary devm_ usage Maciej Fijalkowski
  2024-02-23  7:45   ` Przemek Kitszel
@ 2024-02-23  9:35   ` kernel test robot
  1 sibling, 0 replies; 7+ messages in thread
From: kernel test robot @ 2024-02-23  9:35 UTC (permalink / raw)
  To: Maciej Fijalkowski, intel-wired-lan
  Cc: oe-kbuild-all, netdev, anthony.l.nguyen, magnus.karlsson,
	Maciej Fijalkowski

Hi Maciej,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v6.8-rc5]
[also build test WARNING on linus/master next-20240223]
[cannot apply to tnguy-next-queue/dev-queue tnguy-net-queue/dev-queue horms-ipvs/master]
[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/Maciej-Fijalkowski/ice-do-not-disable-Tx-queues-twice-in-ice_down/20240222-225134
base:   v6.8-rc5
patch link:    https://lore.kernel.org/r/20240222145025.722515-3-maciej.fijalkowski%40intel.com
patch subject: [PATCH iwl-next 2/3] ice: avoid unnecessary devm_ usage
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240223/202402231718.8mWcBppj-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240223/202402231718.8mWcBppj-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/202402231718.8mWcBppj-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/intel/ice/ice_common.c: In function 'ice_update_link_info':
>> drivers/net/ethernet/intel/ice/ice_common.c:3242:32: warning: variable 'hw' set but not used [-Wunused-but-set-variable]
    3242 |                 struct ice_hw *hw;
         |                                ^~
--
   drivers/net/ethernet/intel/ice/ice_ethtool.c: In function 'ice_loopback_test':
>> drivers/net/ethernet/intel/ice/ice_ethtool.c:947:24: warning: variable 'dev' set but not used [-Wunused-but-set-variable]
     947 |         struct device *dev;
         |                        ^~~


vim +/hw +3242 drivers/net/ethernet/intel/ice/ice_common.c

fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3221  
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3222  /**
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3223   * ice_update_link_info - update status of the HW network link
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3224   * @pi: port info structure of the interested logical port
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3225   */
5e24d5984c805c Tony Nguyen            2021-10-07  3226  int ice_update_link_info(struct ice_port_info *pi)
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3227  {
092a33d4031205 Bruce Allan            2019-04-16  3228  	struct ice_link_status *li;
5e24d5984c805c Tony Nguyen            2021-10-07  3229  	int status;
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3230  
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3231  	if (!pi)
d54699e27d506f Tony Nguyen            2021-10-07  3232  		return -EINVAL;
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3233  
092a33d4031205 Bruce Allan            2019-04-16  3234  	li = &pi->phy.link_info;
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3235  
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3236  	status = ice_aq_get_link_info(pi, true, NULL, NULL);
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3237  	if (status)
092a33d4031205 Bruce Allan            2019-04-16  3238  		return status;
092a33d4031205 Bruce Allan            2019-04-16  3239  
092a33d4031205 Bruce Allan            2019-04-16  3240  	if (li->link_info & ICE_AQ_MEDIA_AVAILABLE) {
092a33d4031205 Bruce Allan            2019-04-16  3241  		struct ice_aqc_get_phy_caps_data *pcaps;
092a33d4031205 Bruce Allan            2019-04-16 @3242  		struct ice_hw *hw;
092a33d4031205 Bruce Allan            2019-04-16  3243  
092a33d4031205 Bruce Allan            2019-04-16  3244  		hw = pi->hw;
f8543c3af0dcb2 Maciej Fijalkowski     2024-02-22  3245  		pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL);
092a33d4031205 Bruce Allan            2019-04-16  3246  		if (!pcaps)
d54699e27d506f Tony Nguyen            2021-10-07  3247  			return -ENOMEM;
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3248  
d6730a871e68f1 Anirudh Venkataramanan 2021-03-25  3249  		status = ice_aq_get_phy_caps(pi, false, ICE_AQC_REPORT_TOPO_CAP_MEDIA,
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3250  					     pcaps, NULL);
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3251  
f8543c3af0dcb2 Maciej Fijalkowski     2024-02-22  3252  		kfree(pcaps);
092a33d4031205 Bruce Allan            2019-04-16  3253  	}
092a33d4031205 Bruce Allan            2019-04-16  3254  
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3255  	return status;
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3256  }
fcea6f3da546b9 Anirudh Venkataramanan 2018-03-20  3257  

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

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

* Re: [PATCH iwl-next 2/3] ice: avoid unnecessary devm_ usage
  2024-02-23  7:45   ` Przemek Kitszel
@ 2024-02-23 15:03     ` Maciej Fijalkowski
  0 siblings, 0 replies; 7+ messages in thread
From: Maciej Fijalkowski @ 2024-02-23 15:03 UTC (permalink / raw)
  To: Przemek Kitszel
  Cc: intel-wired-lan, netdev, anthony.l.nguyen, magnus.karlsson

On Fri, Feb 23, 2024 at 08:45:20AM +0100, Przemek Kitszel wrote:
> On 2/22/24 15:50, Maciej Fijalkowski wrote:
> > 1. pcaps are free'd right after AQ routines are done, no need for
> >     devm_'s
> > 2. a test frame for loopback test in ethtool -t is destroyed at the end
> >     of the test so we don't need devm_ here either.
> > 
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > ---
> >   drivers/net/ethernet/intel/ice/ice_common.c  | 23 +++++++++-----------
> >   drivers/net/ethernet/intel/ice/ice_ethtool.c |  4 ++--
> >   2 files changed, 12 insertions(+), 15 deletions(-)
> > 
> 
> nice, thank you!
> 
> BTW, we are committed to using Scope Based Resource Management [1] even
> in the OOT driver, so you could consider that for future IWL submissions
> :)

Thanks for reminding me about this, will do that.

> 
> [1] https://lwn.net/Articles/934679/
> 
> 
> I'm also happy with this as-is too, so:
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

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

end of thread, other threads:[~2024-02-23 15:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-22 14:50 [PATCH iwl-next 0/3] intel: misc improvements Maciej Fijalkowski
2024-02-22 14:50 ` [PATCH iwl-next 1/3] ice: do not disable Tx queues twice in ice_down() Maciej Fijalkowski
2024-02-22 14:50 ` [PATCH iwl-next 2/3] ice: avoid unnecessary devm_ usage Maciej Fijalkowski
2024-02-23  7:45   ` Przemek Kitszel
2024-02-23 15:03     ` Maciej Fijalkowski
2024-02-23  9:35   ` kernel test robot
2024-02-22 14:50 ` [PATCH iwl-next 3/3] ixgbe: pull out stats update to common routines Maciej Fijalkowski

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).