On 29/02/2024 11:27, Siddharth Vadapalli wrote:

On Wed, Feb 28, 2024 at 02:36:55PM +0100, Andrew Lunn wrote:

quoted

quoted

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.

This doesn't really solve the problem as it leaves the question open as to
who will set TI_K3_CPSW_VLAN_AWARE. And the configuration is then fixed at build.

Can you please explain in which scenario the default case does not work for you?
Why would end user want to disable VLAN_AWARE mode?

TRM states
"Transmit packets are NOT modified during switch egress when the VLAN_AWARE bit in the
CPSW_CONTROL_REG register is cleared to 0h. This means that the switch is not in VLAN-aware mode."

The same problem would also apply to cpsw.c and cpsw_new.c correct?

A bit later the driver does this

        /* switch to vlan unaware mode */
        cpsw_ale_control_set(common->ale, HOST_PORT_NUM, ALE_VLAN_AWARE, 1);
        cpsw_ale_control_set(common->ale, HOST_PORT_NUM,
                             ALE_PORT_STATE, ALE_PORT_STATE_FORWARD);

The comment says vlan unaware but code is setting ALE_VLAN_AWARE to 1.
Is the comment wrong?

-- 
cheers,
-roger