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

[Intel-wired-lan] [PATCH] 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.
Removing the second include is OK, because its includers, ixgbe_mbx.c
and ixgbe_82599.c, include ixgbe.h, which includes ixgbe_type.h.

Signed-off-by: Diomidis Spinellis <dds@aueb.gr>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
index bd205306934b..34bc042da4ed 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
@@ -4,8 +4,6 @@
 #ifndef _IXGBE_MBX_H_
 #define _IXGBE_MBX_H_
 
-#include "ixgbe_type.h"
-
 #define IXGBE_VFMAILBOX_SIZE        16 /* 16 32 bit words - 64 bytes */
 
 #define IXGBE_VFMAILBOX             0x002FC
-- 
2.30.2

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

[Tony Nguyen]

On 8/11/2024 4:28 AM, Diomidis Spinellis wrote:

Hi Diomidis,

Thank you for the patch. Could you please include the driver name in the 
title.

i.e. ixgbe: Break include dependency cycle

and also specify the tree you are targeting; presumably '[PATCH iwl-next]'.

> Header ixgbe_type.h includes ixgbe_mbx.h.  Also, header ixgbe_mbx.h
> included ixgbe_type.h, thus introducing a circular dependency.
> Removing the second include is OK, because its includers, ixgbe_mbx.c
> and ixgbe_82599.c, include ixgbe.h, which includes ixgbe_type.h.

While this does resolve the dependency, the preference is to not rely on 
includes from other files to supplement the needed includes. It would be 
better for the headers to explicitly include what they need. Easier said 
than done unfortunately.

I believe for ixgbe_mbx.h, linux/types.h and a forward declaration of 
ixgbe_hw would suffice.

For ixgbe_type.h, it doesn't appear to need ixgbe_mbx.h; though we would 
need add the include to the other files that rely on the include. 
ixgbe_main.c, _sriov.c, _x540.c, and _x550.c are what I'm seeing.

Thanks,
Tony

> Signed-off-by: Diomidis Spinellis <dds@aueb.gr>
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> index bd205306934b..34bc042da4ed 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> @@ -4,8 +4,6 @@
>   #ifndef _IXGBE_MBX_H_
>   #define _IXGBE_MBX_H_
>   
> -#include "ixgbe_type.h"
> -
>   #define IXGBE_VFMAILBOX_SIZE        16 /* 16 32 bit words - 64 bytes */
>   
>   #define IXGBE_VFMAILBOX             0x002FC