Received-SPF: pass (google.com: domain of linux-kernel+bounces-86421-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Thu, 29 Feb 2024 14:57:48 +0530
From: Siddharth Vadapalli <s-vadapalli@ti.com>
To: Andrew Lunn <andrew@lunn.ch>
CC: Siddharth Vadapalli <s-vadapalli@ti.com>, Jiri Pirko <jiri@resnulli.us>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <rogerq@kernel.org>, <vladimir.oltean@nxp.com>,
        <hkallweit1@gmail.com>, <dan.carpenter@linaro.org>, <horms@kernel.org>,
        <yuehaibing@huawei.com>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>,
        <srk@ti.com>
Subject: Re: [PATCH net-next] net: ethernet: ti: am65-cpsw: Add priv-flag for
 Switch VLAN Aware mode
Message-ID: <0004e3d5-0f62-49dc-b51f-5a302006c303@ti.com>
References: <20240227082815.2073826-1-s-vadapalli@ti.com>
 <Zd3YHQRMnv-ZvSWs@nanopsycho>
 <7d1496da-100a-4336-b744-33e843eba930@ti.com>
 <Zd7taFB2nEvtZh8E@nanopsycho>
 <49e531f7-9465-40ea-b604-22a3a7f13d62@ti.com>
 <10287788-614a-4eef-9c9c-a0ef4039b78f@lunn.ch>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <10287788-614a-4eef-9c9c-a0ef4039b78f@lunn.ch>

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.