On Tue, Mar 11, 2025 at 01:38:18PM +0800, Wei Fang wrote: > The command BD ring is used to configure functionality where the > underlying resources may be shared between different entities or being > too large to configure using direct registers (such as lookup tables). > > Because the command BD and table formats of i.MX95 and LS1028A are very > different, the software processing logic is also different. In order to > ensure driver compatibility, struct enetc_si_ops is introduced. This > structure defines some hooks shared by VSI and PSI. Different hardware > driver will register different hooks, For example, setup_cbdr() is used > to initialize the command BD ring, and teardown_cbdr() is used to free > the command BD ring. > > Signed-off-by: Wei Fang <wei.f...@nxp.com> > --- > drivers/net/ethernet/freescale/enetc/enetc.h | 27 +++++++-- > .../net/ethernet/freescale/enetc/enetc4_pf.c | 47 +++++++++++++++- > .../net/ethernet/freescale/enetc/enetc_cbdr.c | 55 +++++++++++++++++-- > .../net/ethernet/freescale/enetc/enetc_pf.c | 13 +++-- > .../net/ethernet/freescale/enetc/enetc_vf.c | 13 +++-- > 5 files changed, 136 insertions(+), 19 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h > b/drivers/net/ethernet/freescale/enetc/enetc.h > index 4ad4eb5c5a74..4ff0957e69be 100644 > --- a/drivers/net/ethernet/freescale/enetc/enetc.h > +++ b/drivers/net/ethernet/freescale/enetc/enetc.h > @@ -8,6 +8,7 @@ > #include <linux/dma-mapping.h> > #include <linux/skbuff.h> > #include <linux/ethtool.h> > +#include <linux/fsl/ntmp.h> > #include <linux/if_vlan.h> > #include <linux/phylink.h> > #include <linux/dim.h> > @@ -266,6 +267,19 @@ struct enetc_platform_info { > const struct enetc_drvdata *data; > }; > > +struct enetc_si; > + > +/* > + * This structure defines the some common hooks for ENETC PSI and VSI. > + * In addition, since VSI only uses the struct enetc_si as its private > + * driver data, so this structure also define some hooks specifically > + * for VSI. For VSI-specific hooks, the format is ‘vf_*()’. > + */ > +struct enetc_si_ops { > + int (*setup_cbdr)(struct enetc_si *si); > + void (*teardown_cbdr)(struct enetc_si *si); > +};
I don't understand the need for si->ops->setup_cbdr() and si->ops->teardown_cbdr()? Doesn't every call site know which kind of SI it is dealing with, and thus it can appropriately call the direct symbol? - the v1 PSI and the VSI call enetc_setup_cbdr() and enetc_teardown_cbdr() - the v4 PSI calls enetc4_setup_cbdr() and enetc4_teardown_cbdr() What benefit is there to making an indirect function call? At least that's what the current code does, I'm not sure if that is the intention.