[Intel-wired-lan] [PATCH] ixgbe: Break include dependency cycle

[Intel-wired-lan] [PATCH] ixgbe: Break include dependency cycle

[Diomidis Spinellis]

Header ixgbe_type.h includes ixgbe_mbx.h.  Also, header
ixgbe_mbx.h included ixgbe_type.h, thus introducing a circular
dependency.

- Remove ixgbe_mbx.h inclusion from ixgbe_type.h.

- ixgbe_mbx.h requires the definition of struct ixgbe_mbx_operations
  so move its definition there. While at it, add missing argument
  identifier names.

- Add required forward structure declarations.

- Include ixgbe_mbx.h in the .c files that need it, for the
  following reasons:

  ixgbe_sriov.c uses ixgbe_check_for_msg
  ixgbe_main.c uses ixgbe_init_mbx_params_pf
  ixgbe_82599.c uses mbx_ops_generic
  ixgbe_x540.c uses mbx_ops_generic
  ixgbe_x550.c uses mbx_ops_generic

Signed-off-by: Diomidis Spinellis <dds@aueb.gr>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h   | 16 +++++++++++++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h  | 15 ++-------------
 drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c  |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c  |  1 +
 7 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
index 283a23150a4d..4aaaea3b5f8f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
@@ -6,6 +6,7 @@
 #include <linux/sched.h>
 
 #include "ixgbe.h"
+#include "ixgbe_mbx.h"
 #include "ixgbe_phy.h"
 
 #define IXGBE_82598_MAX_TX_QUEUES 32
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 8b8404d8c946..c229a26fbbb7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -43,6 +43,7 @@
 #include "ixgbe.h"
 #include "ixgbe_common.h"
 #include "ixgbe_dcb_82599.h"
+#include "ixgbe_mbx.h"
 #include "ixgbe_phy.h"
 #include "ixgbe_sriov.h"
 #include "ixgbe_model.h"
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
index bd205306934b..bf65e82b4c61 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
@@ -4,7 +4,7 @@
 #ifndef _IXGBE_MBX_H_
 #define _IXGBE_MBX_H_
 
-#include "ixgbe_type.h"
+#include <linux/types.h>
 
 #define IXGBE_VFMAILBOX_SIZE        16 /* 16 32 bit words - 64 bytes */
 
@@ -96,6 +96,8 @@ enum ixgbe_pfvf_api_rev {
 #define IXGBE_VF_MBX_INIT_TIMEOUT 2000 /* number of retries on mailbox */
 #define IXGBE_VF_MBX_INIT_DELAY   500  /* microseconds between retries */
 
+struct ixgbe_hw;
+
 int ixgbe_read_mbx(struct ixgbe_hw *, u32 *, u16, u16);
 int ixgbe_write_mbx(struct ixgbe_hw *, u32 *, u16, u16);
 int ixgbe_check_for_msg(struct ixgbe_hw *, u16);
@@ -105,6 +107,18 @@ int ixgbe_check_for_rst(struct ixgbe_hw *, u16);
 void ixgbe_init_mbx_params_pf(struct ixgbe_hw *);
 #endif /* CONFIG_PCI_IOV */
 
+struct ixgbe_mbx_operations {
+	int (*init_params)(struct ixgbe_hw *hw);
+	int (*read)(struct ixgbe_hw *hw, u32 *msg, u16 size, u16 vf_number);
+	int (*write)(struct ixgbe_hw *hw, u32 *msg, u16 size, u16 vf_number);
+	int (*read_posted)(struct ixgbe_hw *hw, u32 *msg, u16 size, u16 mbx_id);
+	int (*write_posted)(struct ixgbe_hw *hw, u32 *msg, u16 size,
+			    u16 mbx_id);
+	int (*check_for_msg)(struct ixgbe_hw *hw, u16 vf_number);
+	int (*check_for_ack)(struct ixgbe_hw *hw, u16 vf_number);
+	int (*check_for_rst)(struct ixgbe_hw *hw, u16 vf_number);
+};
+
 extern const struct ixgbe_mbx_operations mbx_ops_generic;
 
 #endif /* _IXGBE_MBX_H_ */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index e71715f5da22..9631559a5aea 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -18,6 +18,7 @@
 
 #include "ixgbe.h"
 #include "ixgbe_type.h"
+#include "ixgbe_mbx.h"
 #include "ixgbe_sriov.h"
 
 #ifdef CONFIG_PCI_IOV
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 346e3d9114a8..9baccacd02a1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -3601,19 +3601,6 @@ struct ixgbe_phy_info {
 	u32				nw_mng_if_sel;
 };
 
-#include "ixgbe_mbx.h"
-
-struct ixgbe_mbx_operations {
-	int (*init_params)(struct ixgbe_hw *hw);
-	int (*read)(struct ixgbe_hw *, u32 *, u16,  u16);
-	int (*write)(struct ixgbe_hw *, u32 *, u16, u16);
-	int (*read_posted)(struct ixgbe_hw *, u32 *, u16,  u16);
-	int (*write_posted)(struct ixgbe_hw *, u32 *, u16, u16);
-	int (*check_for_msg)(struct ixgbe_hw *, u16);
-	int (*check_for_ack)(struct ixgbe_hw *, u16);
-	int (*check_for_rst)(struct ixgbe_hw *, u16);
-};
-
 struct ixgbe_mbx_stats {
 	u32 msgs_tx;
 	u32 msgs_rx;
@@ -3623,6 +3610,8 @@ struct ixgbe_mbx_stats {
 	u32 rsts;
 };
 
+struct ixgbe_mbx_operations;
+
 struct ixgbe_mbx_info {
 	const struct ixgbe_mbx_operations *ops;
 	struct ixgbe_mbx_stats stats;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
index f1ffa398f6df..81e1df83f136 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
@@ -6,6 +6,7 @@
 #include <linux/sched.h>
 
 #include "ixgbe.h"
+#include "ixgbe_mbx.h"
 #include "ixgbe_phy.h"
 #include "ixgbe_x540.h"
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index a5f644934445..d9a8cf018d3b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -4,6 +4,7 @@
 #include "ixgbe_x540.h"
 #include "ixgbe_type.h"
 #include "ixgbe_common.h"
+#include "ixgbe_mbx.h"
 #include "ixgbe_phy.h"
 
 static int ixgbe_setup_kr_speed_x550em(struct ixgbe_hw *, ixgbe_link_speed);
-- 
2.39.5

Re: [Intel-wired-lan] [PATCH] ixgbe: Break include dependency cycle

[Jacob Keller]

On 10/17/2024 1:58 AM, Diomidis Spinellis wrote:
> Header ixgbe_type.h includes ixgbe_mbx.h.  Also, header
> ixgbe_mbx.h included ixgbe_type.h, thus introducing a circular
> dependency.
> 
> - Remove ixgbe_mbx.h inclusion from ixgbe_type.h.
> 
> - ixgbe_mbx.h requires the definition of struct ixgbe_mbx_operations
>   so move its definition there. While at it, add missing argument
>   identifier names.
> 
> - Add required forward structure declarations.
> 
> - Include ixgbe_mbx.h in the .c files that need it, for the
>   following reasons:
> 
>   ixgbe_sriov.c uses ixgbe_check_for_msg
>   ixgbe_main.c uses ixgbe_init_mbx_params_pf
>   ixgbe_82599.c uses mbx_ops_generic
>   ixgbe_x540.c uses mbx_ops_generic
>   ixgbe_x550.c uses mbx_ops_generic
> 
> Signed-off-by: Diomidis Spinellis <dds@aueb.gr>
> ---

The changes make sense. Which tree was this aimed at? I'd guess as a
cleanup without direct impact that its aimed at next?

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Thanks,
Jake

Re: [Intel-wired-lan] [PATCH] ixgbe: Break include dependency cycle

[Diomidis Spinellis]

On 17-Oct-24 23:36, Jacob Keller wrote:
 >
 > On 10/17/2024 1:58 AM, Diomidis Spinellis wrote:
 >> Header ixgbe_type.h includes ixgbe_mbx.h.  Also, header
 >> ixgbe_mbx.h included ixgbe_type.h, thus introducing a circular
 >> dependency.
 >>
[...]
 >>
 >> Signed-off-by: Diomidis Spinellis <dds@aueb.gr>
 >> ---
 >
 > The changes make sense. Which tree was this aimed at? I'd guess as a
 > cleanup without direct impact that its aimed at next?
 >
 > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Yes, "next" is fine, thank you.

Diomidis - https://www.spinellis.gr

Re: [Intel-wired-lan] [PATCH] ixgbe: Break include dependency cycle

[Romanowski, Rafal]

From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> on behalf of Diomidis Spinellis <dds@aueb.gr>
Sent: Thursday, October 17, 2024 10:58 AM
To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
Cc: intel-wired-lan@lists.osuosl.org <intel-wired-lan@lists.osuosl.org>; Diomidis Spinellis <dds@aueb.gr>
Subject: [Intel-wired-lan] [PATCH] ixgbe: Break include dependency cycle
 
Header ixgbe_type.h includes ixgbe_mbx.h.  Also, header
ixgbe_mbx.h included ixgbe_type.h, thus introducing a circular
dependency.

- Remove ixgbe_mbx.h inclusion from ixgbe_type.h.

- ixgbe_mbx.h requires the definition of struct ixgbe_mbx_operations
  so move its definition there. While at it, add missing argument
  identifier names.

- Add required forward structure declarations.

- Include ixgbe_mbx.h in the .c files that need it, for the
  following reasons:

  ixgbe_sriov.c uses ixgbe_check_for_msg
  ixgbe_main.c uses ixgbe_init_mbx_params_pf
  ixgbe_82599.c uses mbx_ops_generic
  ixgbe_x540.c uses mbx_ops_generic
  ixgbe_x550.c uses mbx_ops_generic

Signed-off-by: Diomidis Spinellis <dds@aueb.gr>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h   | 16 +++++++++++++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h  | 15 ++-------------
 drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c  |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c  |  1 +
 7 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
index 283a23150a4d..4aaaea3b5f8f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
@@ -6,6 +6,7 @@
 #include <linux/sched.h>
 
 #include "ixgbe.h"
+#include "ixgbe_mbx.h"
 #include "ixgbe_phy.h"
 
 #define IXGBE_82598_MAX_TX_QUEUES 32
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 8b8404d8c946..c229a26fbbb7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -43,6 +43,7 @@
 #include "ixgbe.h"
 #include "ixgbe_common.h"
 #include "ixgbe_dcb_82599.h"
+#include "ixgbe_mbx.h"
 #include "ixgbe_phy.h"
 #include "ixgbe_sriov.h"
 #include "ixgbe_model.h"
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
index bd205306934b..bf65e82b4c61 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
@@ -4,7 +4,7 @@
 #ifndef _IXGBE_MBX_H_
 #define _IXGBE_MBX_H_
 
-#include "ixgbe_type.h"
+#include <linux/types.h>
 
 #define IXGBE_VFMAILBOX_SIZE        16 /* 16 32 bit words - 64 bytes */
 
@@ -96,6 +96,8 @@ enum ixgbe_pfvf_api_rev {
 #define IXGBE_VF_MBX_INIT_TIMEOUT 2000 /* number of retries on mailbox */
 #define IXGBE_VF_MBX_INIT_DELAY   500  /* microseconds between retries */
 
+struct ixgbe_hw;
+
 int ixgbe_read_mbx(struct ixgbe_hw *, u32 *, u16, u16);
 int ixgbe_write_mbx(struct ixgbe_hw *, u32 *, u16, u16);
 int ixgbe_check_for_msg(struct ixgbe_hw *, u16);
@@ -105,6 +107,18 @@ int ixgbe_check_for_rst(struct ixgbe_hw *, u16);
 void ixgbe_init_mbx_params_pf(struct ixgbe_hw *);
 #endif /* CONFIG_PCI_IOV */
 
+struct ixgbe_mbx_operations {
+       int (*init_params)(struct ixgbe_hw *hw);
+       int (*read)(struct ixgbe_hw *hw, u32 *msg, u16 size, u16 vf_number);
+       int (*write)(struct ixgbe_hw *hw, u32 *msg, u16 size, u16 vf_number);
+       int (*read_posted)(struct ixgbe_hw *hw, u32 *msg, u16 size, u16 mbx_id);
+       int (*write_posted)(struct ixgbe_hw *hw, u32 *msg, u16 size,
+                           u16 mbx_id);
+       int (*check_for_msg)(struct ixgbe_hw *hw, u16 vf_number);
+       int (*check_for_ack)(struct ixgbe_hw *hw, u16 vf_number);
+       int (*check_for_rst)(struct ixgbe_hw *hw, u16 vf_number);
+};
+
 extern const struct ixgbe_mbx_operations mbx_ops_generic;
 
 #endif /* _IXGBE_MBX_H_ */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index e71715f5da22..9631559a5aea 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -18,6 +18,7 @@
 
 #include "ixgbe.h"
 #include "ixgbe_type.h"
+#include "ixgbe_mbx.h"
 #include "ixgbe_sriov.h"
 
 #ifdef CONFIG_PCI_IOV
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 346e3d9114a8..9baccacd02a1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -3601,19 +3601,6 @@ struct ixgbe_phy_info {
         u32                             nw_mng_if_sel;
 };
 
-#include "ixgbe_mbx.h"
-
-struct ixgbe_mbx_operations {
-       int (*init_params)(struct ixgbe_hw *hw);
-       int (*read)(struct ixgbe_hw *, u32 *, u16,  u16);
-       int (*write)(struct ixgbe_hw *, u32 *, u16, u16);
-       int (*read_posted)(struct ixgbe_hw *, u32 *, u16,  u16);
-       int (*write_posted)(struct ixgbe_hw *, u32 *, u16, u16);
-       int (*check_for_msg)(struct ixgbe_hw *, u16);
-       int (*check_for_ack)(struct ixgbe_hw *, u16);
-       int (*check_for_rst)(struct ixgbe_hw *, u16);
-};
-
 struct ixgbe_mbx_stats {
         u32 msgs_tx;
         u32 msgs_rx;
@@ -3623,6 +3610,8 @@ struct ixgbe_mbx_stats {
         u32 rsts;
 };
 
+struct ixgbe_mbx_operations;
+
 struct ixgbe_mbx_info {
         const struct ixgbe_mbx_operations *ops;
         struct ixgbe_mbx_stats stats;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
index f1ffa398f6df..81e1df83f136 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
@@ -6,6 +6,7 @@
 #include <linux/sched.h>
 
 #include "ixgbe.h"
+#include "ixgbe_mbx.h"
 #include "ixgbe_phy.h"
 #include "ixgbe_x540.h"
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index a5f644934445..d9a8cf018d3b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -4,6 +4,7 @@
 #include "ixgbe_x540.h"
 #include "ixgbe_type.h"
 #include "ixgbe_common.h"
+#include "ixgbe_mbx.h"
 #include "ixgbe_phy.h"
 
 static int ixgbe_setup_kr_speed_x550em(struct ixgbe_hw *, ixgbe_link_speed);
--
2.39.5



Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>