Re: [PATCH net-next] net: ethernet: ti: am65-cpsw: Add priv-flag for Switch VLAN Aware mode

From: Siddharth Vadapalli
Date: Thu Feb 29 2024 - 04:28:54 EST


On Wed, Feb 28, 2024 at 02:36:55PM +0100, Andrew Lunn wrote:
> > What if there is no kernel behavior associated with it? How can it be mimicked
> > then?
>
> Simple. Implement the feature in software in the kernel for
> everybody. Then offload it to your hardware.
>
> Your hardware is an accelerator. You use it to accelerate what linux
> can already do. If Linux does not have the feature your accelerator
> has, that accelerator feature goes unused.

Is it acceptable to have a macro in the Ethernet Driver to conditionally
disable/enable the feature (via setting the corresponding bit in the
register)?

The current implementation is:

/* Control register */
writel(AM65_CPSW_CTL_P0_ENABLE | AM65_CPSW_CTL_P0_TX_CRC_REMOVE |
AM65_CPSW_CTL_VLAN_AWARE | AM65_CPSW_CTL_P0_RX_PAD,
common->cpsw_base + AM65_CPSW_REG_CTL);

which sets the "AM65_CPSW_CTL_VLAN_AWARE" bit by default.

Could it be changed to:

#define TI_K3_CPSW_VLAN_AWARE 1

...

/* Control register */
val = AM65_CPSW_CTL_P0_ENABLE | AM65_CPSW_CTL_P0_TX_CRC_REMOVE |
AM65_CPSW_CTL_P0_RX_PAD;

#ifdef TI_K3_CPSW_VLAN_AWARE
val |= AM65_CPSW_CTL_VLAN_AWARE;
#endif

writel(val, common->cpsw_base + AM65_CPSW_REG_CTL);

Since no additional configuration is necessary to disable/enable the
functionality except clearing/setting a bit in a register, I am unsure of
the implementation for the offloading part being suggested. Please let me
know if the above implementation is an acceptable alternative.

Regards,
Siddharth.