Received-SPF: pass (google.com: domain of linux-kernel+bounces-144624-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <bc0e05c5-11a8-4519-b50d-04dabd6e5999@ti.com>
Date: Mon, 15 Apr 2024 12:26:48 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v3 3/3] net: ti: icssg-prueth: Add support for
 ICSSG switch firmware
To: Andrew Davis <afd@ti.com>, MD Danish Anwar <danishanwar@ti.com>,
        Diogo Ivo
	<diogo.ivo@siemens.com>, Rob Herring <robh@kernel.org>,
        Dan Carpenter
	<dan.carpenter@linaro.org>,
        Jan Kiszka <jan.kiszka@siemens.com>, Simon Horman
	<horms@kernel.org>,
        Andrew Lunn <andrew@lunn.ch>,
        Wolfram Sang
	<wsa+renesas@sang-engineering.com>,
        Arnd Bergmann <arnd@arndb.de>, Vignesh
 Raghavendra <vigneshr@ti.com>,
        Vladimir Oltean <vladimir.oltean@nxp.com>,
        Roger Quadros <rogerq@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
        Jakub
 Kicinski <kuba@kernel.org>, Eric Dumazet <edumazet@google.com>,
        "David S.
 Miller" <davem@davemloft.net>
CC: <linux-arm-kernel@lists.infradead.org>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <srk@ti.com>, <r-gunasekaran@ti.com>
References: <20240327114054.1907278-1-danishanwar@ti.com>
 <20240327114054.1907278-4-danishanwar@ti.com>
 <cb13da4a-13c9-409a-a813-0ac852062163@ti.com>
Content-Language: en-US
From: "Anwar, Md Danish" <a0501179@ti.com>
In-Reply-To: <cb13da4a-13c9-409a-a813-0ac852062163@ti.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit

Hi Andrew,

On 4/13/2024 12:22 AM, Andrew Davis wrote:
> On 3/27/24 6:40 AM, MD Danish Anwar wrote:
>> Add support for ICSSG switch firmware using existing Dual EMAC driver
>> with switchdev.
>>
>> Limitations:
>> VLAN offloading is limited to 0-256 IDs.
>> MDB/FDB static entries are limited to 511 entries and different FDBs can
>> hash to same bucket and thus may not completely offloaded
>>
>> Switch mode requires loading of new firmware into ICSSG cores. This
>> means interfaces have to taken down and then reconfigured to switch
>> mode.
>>
>> Example assuming ETH1 and ETH2 as ICSSG2 interfaces:
>>
>> Switch to ICSSG Switch mode:
>>   ip link set dev eth1 down
>>   ip link set dev eth2 down
>>   ip link add name br0 type bridge
>>   ip link set dev eth1 master br0
>>   ip link set dev eth2 master br0
>>   ip link set dev br0 up
>>   ip link set dev eth1 up
>>   ip link set dev eth2 up
>>   bridge vlan add dev br0 vid 1 pvid untagged self
>>
>> Going back to Dual EMAC mode:
>>
>>   ip link set dev br0 down
>>   ip link set dev eth1 nomaster
>>   ip link set dev eth2 nomaster
>>   ip link set dev eth1 down
>>   ip link set dev eth2 down
>>   ip link del name br0 type bridge
>>   ip link set dev eth1 up
>>   ip link set dev eth2 up
>>
>> By default, Dual EMAC firmware is loaded, and can be changed to switch
>> mode by above steps
>>
> 
> This was asked before, maybe I missed the answer, but why do we
> default to Dual-EMAC firmware? I remember when I was working on
> the original ICSS-ETH driver, we started with the Dual-EMAC
> firmware as the switch firmware was not ready yet (and EMAC mode
> was easier). Now that we have both available, if we just use Switch
> firmwar by default, what would we lose? Seems that would solve
> the issues with re-loading firmware at runtime (configuration loss
> and dropping packets, etc..).
> 

We can start the driver with either Dual-EMAC firmware or SWITCH
firmware. But the problem lies in switching between these two firmwares.
For switching to / from Dual-EMAC and switch firmwares we need to stop
the cores and that is where we previously used to bring down the
interfaces, switch firmware and bring it up again. But as discussed on
this thread, I can now do the same without bringing interfaces up /
down. We'll just need to stop the cores and change firmware this will
also result in preserving the configuration. There will be packet loss
but that will not be a big concern as Andrew L. pointed out.

Currently we are starting in Dual-EMAC mode as by default the interfaces
are not needed to forward packets. They are supposed to act as
individual ports. Port to port forwarding is not needed. Only when user
adds a bridge and starts forwarding we switch to Switch mode and load
different firmware so that packet forwarding can happen in firmware.
That is why currently we are starting Dual-EMAC mode and then switching
to firmware.

If we use switch firmware by default, we will not be able to use
individual ports separately and any data sent to one port will be
forwarded to the second port.

I will be posting v4 soon and I will describe all the details on how to
use and switch between different modes in the cover letter.

> Andrew
> 

[ ... ]

>>     static const struct prueth_pdata am64x_icssg_pdata = {
>>       .fdqring_mode = K3_RINGACC_RING_MODE_RING,
>> +    .switch_mode = 1,
>>   };
>>     static const struct of_device_id prueth_dt_match[] = {

-- 
Thanks and Regards,
Md Danish Anwar