[PATCH net-next v3 1/2] net: stmmac: Refactor VLAN implementation

Ng, Boon Khai boon.khai.ng at altera.com
Mon Apr 21 09:40:14 PDT 2025


 
> Rename dwmac_vlan_ops to dwmac4_vlan_ops will be better,
> just like dwmac4_desc_ops/dwmac4_dma_ops

Hi Furong thanks for the feedback, 

This dwmac_vlan_ops is defined the same at dwmac4, dwmac510, 
and dwxgmac210, thus consolidate them in the 
same ops: dwmac_vlan_ops.

> dwxlgmac2_vlan_ops looks redundant here, another new struct contains
> totally identical members.
> 
> stmmac_do_void_callback()/stmmac_do_callback() handles NULL function
> pointers so good, we can leave the un-implemented functions as NULL.
> 
> Are you trying to avoid something undefined here?

Nope, since dwxlgmac2_vlan_ops does not hold the same ops with
dwxgmac210, dwmac4, dwmac510, this is not newly enabled, just move
over from the initial implementation on the ops assignment.

> 
> It is a good practice to only keep inside the header those definitions
> which are truly exported by stmmac_vlan.c towards external callers.
> That means those #defines which are only used within stmmac_vlan.c
> shouldn't be here, but inside stmmac_vlan.c file.

I prefer to keep all #define directives in the header file to enhance
 code readability and debugging efficiency by providing a single
reference point for developers.

Regards,
Boon Khai.



More information about the linux-arm-kernel mailing list