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