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
Signed-off-by: MD Danish Anwar <[email protected]>
---
drivers/net/ethernet/ti/Kconfig | 1 +
drivers/net/ethernet/ti/Makefile | 3 +-
drivers/net/ethernet/ti/icssg/icssg_config.c | 136 +++++++++++--
drivers/net/ethernet/ti/icssg/icssg_config.h | 7 +
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 198 ++++++++++++++++++-
5 files changed, 332 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 1530d13984d4..9b5f5f680b35 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -188,6 +188,7 @@ config TI_ICSSG_PRUETH
select TI_ICSS_IEP
select TI_K3_CPPI_DESC_POOL
depends on PRU_REMOTEPROC
+ depends on NET_SWITCHDEV
depends on ARCH_K3 && OF && TI_K3_UDMA_GLUE_LAYER
depends on PTP_1588_CLOCK_OPTIONAL
help
diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
index d8590304f3df..d295bded7a32 100644
--- a/drivers/net/ethernet/ti/Makefile
+++ b/drivers/net/ethernet/ti/Makefile
@@ -38,5 +38,6 @@ icssg-prueth-y := icssg/icssg_prueth.o \
icssg/icssg_config.o \
icssg/icssg_mii_cfg.o \
icssg/icssg_stats.o \
- icssg/icssg_ethtool.o
+ icssg/icssg_ethtool.o \
+ icssg/icssg_switchdev.o
obj-$(CONFIG_TI_ICSS_IEP) += icssg/icss_iep.o
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
index 970e6ef9ba64..5b2064d1b03b 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
@@ -105,28 +105,49 @@ static const struct map hwq_map[2][ICSSG_NUM_OTHER_QUEUES] = {
},
};
+static void icssg_config_mii_init_switch(struct prueth_emac *emac)
+{
+ struct prueth *prueth = emac->prueth;
+ int mii = prueth_emac_slice(emac);
+ u32 txcfg_reg, pcnt_reg, txcfg;
+ struct regmap *mii_rt;
+
+ mii_rt = prueth->mii_rt;
+
+ txcfg_reg = (mii == ICSS_MII0) ? PRUSS_MII_RT_TXCFG0 :
+ PRUSS_MII_RT_TXCFG1;
+ pcnt_reg = (mii == ICSS_MII0) ? PRUSS_MII_RT_RX_PCNT0 :
+ PRUSS_MII_RT_RX_PCNT1;
+
+ txcfg = PRUSS_MII_RT_TXCFG_TX_ENABLE |
+ PRUSS_MII_RT_TXCFG_TX_AUTO_PREAMBLE |
+ PRUSS_MII_RT_TXCFG_TX_IPG_WIRE_CLK_EN;
+
+ if (emac->phy_if == PHY_INTERFACE_MODE_MII && mii == ICSS_MII1)
+ txcfg |= PRUSS_MII_RT_TXCFG_TX_MUX_SEL;
+ else if (emac->phy_if != PHY_INTERFACE_MODE_MII && mii == ICSS_MII0)
+ txcfg |= PRUSS_MII_RT_TXCFG_TX_MUX_SEL;
+
+ regmap_write(mii_rt, txcfg_reg, txcfg);
+ regmap_write(mii_rt, pcnt_reg, 0x1);
+}
+
static void icssg_config_mii_init(struct prueth_emac *emac)
{
- u32 rxcfg, txcfg, rxcfg_reg, txcfg_reg, pcnt_reg;
struct prueth *prueth = emac->prueth;
int slice = prueth_emac_slice(emac);
+ u32 txcfg, txcfg_reg, pcnt_reg;
struct regmap *mii_rt;
mii_rt = prueth->mii_rt;
- rxcfg_reg = (slice == ICSS_MII0) ? PRUSS_MII_RT_RXCFG0 :
- PRUSS_MII_RT_RXCFG1;
txcfg_reg = (slice == ICSS_MII0) ? PRUSS_MII_RT_TXCFG0 :
PRUSS_MII_RT_TXCFG1;
pcnt_reg = (slice == ICSS_MII0) ? PRUSS_MII_RT_RX_PCNT0 :
PRUSS_MII_RT_RX_PCNT1;
- rxcfg = MII_RXCFG_DEFAULT;
txcfg = MII_TXCFG_DEFAULT;
- if (slice == ICSS_MII1)
- rxcfg |= PRUSS_MII_RT_RXCFG_RX_MUX_SEL;
-
/* In MII mode TX lines swapped inside ICSSG, so TX_MUX_SEL cfg need
* to be swapped also comparing to RGMII mode.
*/
@@ -135,7 +156,6 @@ static void icssg_config_mii_init(struct prueth_emac *emac)
else if (emac->phy_if != PHY_INTERFACE_MODE_MII && slice == ICSS_MII1)
txcfg |= PRUSS_MII_RT_TXCFG_TX_MUX_SEL;
- regmap_write(mii_rt, rxcfg_reg, rxcfg);
regmap_write(mii_rt, txcfg_reg, txcfg);
regmap_write(mii_rt, pcnt_reg, 0x1);
}
@@ -249,6 +269,66 @@ static int emac_r30_is_done(struct prueth_emac *emac)
return 1;
}
+static int prueth_switch_buffer_setup(struct prueth_emac *emac)
+{
+ struct icssg_buffer_pool_cfg __iomem *bpool_cfg;
+ struct icssg_rxq_ctx __iomem *rxq_ctx;
+ struct prueth *prueth = emac->prueth;
+ int slice = prueth_emac_slice(emac);
+ u32 addr;
+ int i;
+
+ addr = lower_32_bits(prueth->msmcram.pa);
+ if (slice)
+ addr += PRUETH_NUM_BUF_POOLS * PRUETH_EMAC_BUF_POOL_SIZE;
+
+ if (addr % SZ_64K) {
+ dev_warn(prueth->dev, "buffer pool needs to be 64KB aligned\n");
+ return -EINVAL;
+ }
+
+ bpool_cfg = emac->dram.va + BUFFER_POOL_0_ADDR_OFFSET;
+ /* workaround for f/w bug. bpool 0 needs to be initilalized */
+ for (i = 0; i < PRUETH_NUM_BUF_POOLS; i++) {
+ writel(addr, &bpool_cfg[i].addr);
+ writel(PRUETH_EMAC_BUF_POOL_SIZE, &bpool_cfg[i].len);
+ addr += PRUETH_EMAC_BUF_POOL_SIZE;
+ }
+
+ if (!slice)
+ addr += PRUETH_NUM_BUF_POOLS * PRUETH_EMAC_BUF_POOL_SIZE;
+ else
+ addr += PRUETH_SW_NUM_BUF_POOLS_HOST * PRUETH_SW_BUF_POOL_SIZE_HOST;
+
+ for (i = PRUETH_NUM_BUF_POOLS;
+ i < 2 * PRUETH_SW_NUM_BUF_POOLS_HOST + PRUETH_NUM_BUF_POOLS;
+ i++) {
+ /* The driver only uses first 4 queues per PRU so only initialize them */
+ if (i % PRUETH_SW_NUM_BUF_POOLS_HOST < PRUETH_SW_NUM_BUF_POOLS_PER_PRU) {
+ writel(addr, &bpool_cfg[i].addr);
+ writel(PRUETH_SW_BUF_POOL_SIZE_HOST, &bpool_cfg[i].len);
+ addr += PRUETH_SW_BUF_POOL_SIZE_HOST;
+ } else {
+ writel(0, &bpool_cfg[i].addr);
+ writel(0, &bpool_cfg[i].len);
+ }
+ }
+
+ if (!slice)
+ addr += PRUETH_SW_NUM_BUF_POOLS_HOST * PRUETH_SW_BUF_POOL_SIZE_HOST;
+ else
+ addr += PRUETH_EMAC_RX_CTX_BUF_SIZE;
+
+ rxq_ctx = emac->dram.va + HOST_RX_Q_PRE_CONTEXT_OFFSET;
+ for (i = 0; i < 3; i++)
+ writel(addr, &rxq_ctx->start[i]);
+
+ addr += PRUETH_EMAC_RX_CTX_BUF_SIZE;
+ writel(addr - SZ_2K, &rxq_ctx->end);
+
+ return 0;
+}
+
static int prueth_emac_buffer_setup(struct prueth_emac *emac)
{
struct icssg_buffer_pool_cfg __iomem *bpool_cfg;
@@ -325,13 +405,41 @@ static void icssg_init_emac_mode(struct prueth *prueth)
icssg_class_set_host_mac_addr(prueth->miig_rt, mac);
}
+static void icssg_init_switch_mode(struct prueth *prueth)
+{
+ u32 addr = prueth->shram.pa + EMAC_ICSSG_SWITCH_DEFAULT_VLAN_TABLE_OFFSET;
+ int i;
+
+ if (prueth->emacs_initialized)
+ return;
+
+ /* Set VLAN TABLE address base */
+ regmap_update_bits(prueth->miig_rt, FDB_GEN_CFG1, SMEM_VLAN_OFFSET_MASK,
+ addr << SMEM_VLAN_OFFSET);
+ /* Set enable VLAN aware mode, and FDBs for all PRUs */
+ regmap_write(prueth->miig_rt, FDB_GEN_CFG2, FDB_EN_ALL);
+ prueth->vlan_tbl = (struct prueth_vlan_tbl __force *)(prueth->shram.va +
+ EMAC_ICSSG_SWITCH_DEFAULT_VLAN_TABLE_OFFSET);
+ for (i = 0; i < SZ_4K - 1; i++) {
+ prueth->vlan_tbl[i].fid = i;
+ prueth->vlan_tbl[i].fid_c1 = 0;
+ }
+
+ if (prueth->hw_bridge_dev)
+ icssg_class_set_host_mac_addr(prueth->miig_rt, prueth->hw_bridge_dev->dev_addr);
+ icssg_set_pvid(prueth, prueth->default_vlan, PRUETH_PORT_HOST);
+}
+
int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
{
void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
struct icssg_flow_cfg __iomem *flow_cfg;
int ret;
- icssg_init_emac_mode(prueth);
+ if (prueth->is_switch_mode)
+ icssg_init_switch_mode(prueth);
+ else
+ icssg_init_emac_mode(prueth);
memset_io(config, 0, TAS_GATE_MASK_LIST0);
icssg_miig_queues_init(prueth, slice);
@@ -345,7 +453,10 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
regmap_update_bits(prueth->miig_rt, ICSSG_CFG_OFFSET,
ICSSG_CFG_DEFAULT, ICSSG_CFG_DEFAULT);
icssg_miig_set_interface_mode(prueth->miig_rt, slice, emac->phy_if);
- icssg_config_mii_init(emac);
+ if (prueth->is_switch_mode)
+ icssg_config_mii_init_switch(emac);
+ else
+ icssg_config_mii_init(emac);
icssg_config_ipg(emac);
icssg_update_rgmii_cfg(prueth->miig_rt, emac);
@@ -368,7 +479,10 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
writeb(0, config + SPL_PKT_DEFAULT_PRIORITY);
writeb(0, config + QUEUE_NUM_UNTAGGED);
- ret = prueth_emac_buffer_setup(emac);
+ if (prueth->is_switch_mode)
+ ret = prueth_switch_buffer_setup(emac);
+ else
+ ret = prueth_emac_buffer_setup(emac);
if (ret)
return ret;
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
index 0d5d5d253b7a..cc923f1d4387 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
@@ -35,6 +35,13 @@ struct icssg_flow_cfg {
(2 * (PRUETH_EMAC_BUF_POOL_SIZE * PRUETH_NUM_BUF_POOLS + \
PRUETH_EMAC_RX_CTX_BUF_SIZE * 2))
+#define PRUETH_SW_BUF_POOL_SIZE_HOST SZ_4K
+#define PRUETH_SW_NUM_BUF_POOLS_HOST 8
+#define PRUETH_SW_NUM_BUF_POOLS_PER_PRU 4
+#define MSMC_RAM_SIZE_SWITCH_MODE \
+ (MSMC_RAM_SIZE + \
+ (2 * PRUETH_SW_BUF_POOL_SIZE_HOST * PRUETH_SW_NUM_BUF_POOLS_HOST))
+
#define PRUETH_SWITCH_FDB_MASK ((SIZE_OF_FDB / NUMBER_OF_FDB_BUCKET_ENTRIES) - 1)
struct icssg_rxq_ctx {
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 9168ab3c4b9e..725b5de05e00 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -27,6 +27,7 @@
#include <linux/remoteproc/pruss.h>
#include <linux/regmap.h>
#include <linux/remoteproc.h>
+#include <net/switchdev.h>
#include "icssg_prueth.h"
#include "icssg_mii_rt.h"
@@ -54,6 +55,10 @@
#define prueth_napi_to_emac(napi) container_of(napi, struct prueth_emac, napi_rx)
+#define DEFAULT_VID 1
+#define DEFAULT_PORT_MASK 1
+#define DEFAULT_UNTAG_MASK 1
+
/* CTRLMMR_ICSSG_RGMII_CTRL register bits */
#define ICSSG_CTRL_RGMII_ID_MODE BIT(24)
@@ -558,6 +563,8 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
} else {
/* send the filled skb up the n/w stack */
skb_put(skb, pkt_len);
+ if (emac->prueth->is_switch_mode)
+ skb->offload_fwd_mark = emac->offload_fwd_mark;
skb->protocol = eth_type_trans(skb, ndev);
napi_gro_receive(&emac->napi_rx, skb);
ndev->stats.rx_bytes += pkt_len;
@@ -890,6 +897,19 @@ struct icssg_firmwares {
char *txpru;
};
+static struct icssg_firmwares icssg_switch_firmwares[] = {
+ {
+ .pru = "ti-pruss/am65x-sr2-pru0-prusw-fw.elf",
+ .rtu = "ti-pruss/am65x-sr2-rtu0-prusw-fw.elf",
+ .txpru = "ti-pruss/am65x-sr2-txpru0-prusw-fw.elf",
+ },
+ {
+ .pru = "ti-pruss/am65x-sr2-pru1-prusw-fw.elf",
+ .rtu = "ti-pruss/am65x-sr2-rtu1-prusw-fw.elf",
+ .txpru = "ti-pruss/am65x-sr2-txpru1-prusw-fw.elf",
+ }
+};
+
static struct icssg_firmwares icssg_emac_firmwares[] = {
{
.pru = "ti-pruss/am65x-sr2-pru0-prueth-fw.elf",
@@ -909,7 +929,10 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
struct device *dev = prueth->dev;
int slice, ret;
- firmwares = icssg_emac_firmwares;
+ if (prueth->is_switch_mode)
+ firmwares = icssg_switch_firmwares;
+ else
+ firmwares = icssg_emac_firmwares;
slice = prueth_emac_slice(emac);
if (slice < 0) {
@@ -1411,6 +1434,21 @@ static int emac_ndo_open(struct net_device *ndev)
queue_work(system_long_wq, &emac->stats_work.work);
+ if (prueth->is_switch_mode) {
+ icssg_fdb_add_del(emac, eth_stp_addr, prueth->default_vlan,
+ ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
+ ICSSG_FDB_ENTRY_P1_MEMBERSHIP |
+ ICSSG_FDB_ENTRY_P2_MEMBERSHIP |
+ ICSSG_FDB_ENTRY_BLOCK,
+ true);
+ icssg_vtbl_modify(emac, emac->port_vlan | DEFAULT_VID,
+ BIT(emac->port_id) | DEFAULT_PORT_MASK,
+ BIT(emac->port_id) | DEFAULT_UNTAG_MASK,
+ true);
+ icssg_set_pvid(emac->prueth, emac->port_vlan, emac->port_id);
+ emac_set_port_state(emac, ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE);
+ }
+
return 0;
reset_tx_chan:
@@ -1945,6 +1983,148 @@ static void prueth_put_cores(struct prueth *prueth, int slice)
pru_rproc_put(prueth->pru[slice]);
}
+static void prueth_offload_fwd_mark_update(struct prueth *prueth)
+{
+ int set_val = 0;
+ int i;
+
+ if (prueth->br_members == (PRUETH_PORT_MII0 | PRUETH_PORT_MII1))
+ set_val = 1;
+
+ dev_dbg(prueth->dev, "set offload_fwd_mark %d\n", set_val);
+
+ for (i = PRUETH_MAC0; i < PRUETH_NUM_MACS; i++) {
+ struct prueth_emac *emac = prueth->emac[i];
+
+ if (!emac || !emac->ndev)
+ continue;
+
+ emac->offload_fwd_mark = set_val;
+ }
+}
+
+bool prueth_dev_check(const struct net_device *ndev)
+{
+ if (ndev->netdev_ops == &emac_netdev_ops && netif_running(ndev)) {
+ struct prueth_emac *emac = netdev_priv(ndev);
+
+ return emac->prueth->is_switch_mode;
+ }
+
+ return false;
+}
+
+static int prueth_netdevice_port_link(struct net_device *ndev,
+ struct net_device *br_ndev,
+ struct netlink_ext_ack *extack)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth *prueth = emac->prueth;
+ int err;
+
+ if (!prueth->br_members) {
+ prueth->hw_bridge_dev = br_ndev;
+ } else {
+ /* This is adding the port to a second bridge, this is
+ * unsupported
+ */
+ if (prueth->hw_bridge_dev != br_ndev)
+ return -EOPNOTSUPP;
+ }
+
+ err = switchdev_bridge_port_offload(br_ndev, ndev, emac,
+ &prueth->prueth_switchdev_nb,
+ &prueth->prueth_switchdev_bl_nb,
+ false, extack);
+ if (err)
+ return err;
+
+ prueth->br_members |= BIT(emac->port_id);
+
+ if (prueth->br_members & BIT(PRUETH_PORT_MII0) &&
+ prueth->br_members & BIT(PRUETH_PORT_MII1)) {
+ prueth->is_switch_mode = true;
+ prueth->default_vlan = 1;
+ emac->port_vlan = prueth->default_vlan;
+ }
+
+ prueth_offload_fwd_mark_update(prueth);
+
+ return NOTIFY_DONE;
+}
+
+static void prueth_netdevice_port_unlink(struct net_device *ndev)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth *prueth = emac->prueth;
+
+ prueth->br_members &= ~BIT(emac->port_id);
+
+ prueth->is_switch_mode = false;
+ emac->port_vlan = 0;
+
+ prueth_offload_fwd_mark_update(prueth);
+
+ if (!prueth->br_members)
+ prueth->hw_bridge_dev = NULL;
+}
+
+/* netdev notifier */
+static int prueth_netdevice_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(ptr);
+ struct net_device *ndev = netdev_notifier_info_to_dev(ptr);
+ struct netdev_notifier_changeupper_info *info;
+ int ret = NOTIFY_DONE;
+
+ if (ndev->netdev_ops != &emac_netdev_ops)
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case NETDEV_CHANGEUPPER:
+ info = ptr;
+
+ if (netif_is_bridge_master(info->upper_dev)) {
+ if (info->linking)
+ ret = prueth_netdevice_port_link(ndev, info->upper_dev, extack);
+ else
+ prueth_netdevice_port_unlink(ndev);
+ }
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ return notifier_from_errno(ret);
+}
+
+static int prueth_register_notifiers(struct prueth *prueth)
+{
+ int ret = 0;
+
+ prueth->prueth_netdevice_nb.notifier_call = &prueth_netdevice_event;
+ ret = register_netdevice_notifier(&prueth->prueth_netdevice_nb);
+ if (ret) {
+ dev_err(prueth->dev, "can't register netdevice notifier\n");
+ return ret;
+ }
+
+ ret = prueth_switchdev_register_notifiers(prueth);
+ if (ret)
+ unregister_netdevice_notifier(&prueth->prueth_netdevice_nb);
+
+ return ret;
+}
+
+static void prueth_unregister_notifiers(struct prueth *prueth)
+{
+ prueth_switchdev_unregister_notifiers(prueth);
+ unregister_netdevice_notifier(&prueth->prueth_netdevice_nb);
+}
+
+static const struct of_device_id prueth_dt_match[];
+
static int prueth_probe(struct platform_device *pdev)
{
struct device_node *eth_node, *eth_ports_node;
@@ -2072,6 +2252,10 @@ static int prueth_probe(struct platform_device *pdev)
}
msmc_ram_size = MSMC_RAM_SIZE;
+ prueth->is_switchmode_supported = prueth->pdata.switch_mode;
+ if (prueth->is_switchmode_supported)
+ msmc_ram_size = MSMC_RAM_SIZE_SWITCH_MODE;
+
/* NOTE: FW bug needs buffer base to be 64KB aligned */
prueth->msmcram.va =
@@ -2167,6 +2351,14 @@ static int prueth_probe(struct platform_device *pdev)
phy_attached_info(prueth->emac[PRUETH_MAC1]->ndev->phydev);
}
+ if (prueth->is_switchmode_supported) {
+ ret = prueth_register_notifiers(prueth);
+ if (ret)
+ goto netdev_unregister;
+
+ sprintf(prueth->switch_id, "%s", dev_name(dev));
+ }
+
dev_info(dev, "TI PRU ethernet driver initialized: %s EMAC mode\n",
(!eth0_node || !eth1_node) ? "single" : "dual");
@@ -2236,6 +2428,8 @@ static void prueth_remove(struct platform_device *pdev)
struct device_node *eth_node;
int i;
+ prueth_unregister_notifiers(prueth);
+
for (i = 0; i < PRUETH_NUM_MACS; i++) {
if (!prueth->registered_netdevs[i])
continue;
@@ -2333,10 +2527,12 @@ static const struct dev_pm_ops prueth_dev_pm_ops = {
static const struct prueth_pdata am654_icssg_pdata = {
.fdqring_mode = K3_RINGACC_RING_MODE_MESSAGE,
.quirk_10m_link_issue = 1,
+ .switch_mode = 1,
};
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[] = {
--
2.34.1
On Wed, Mar 27, 2024 at 05:10:54PM +0530, 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.
Patch 0/3 does not say this. It just shows the interfaces being added
to the bridge. There should not be any need to down the interfaces.
> 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
I keep asking this, so it would be good to explain it in the commit
message. What configuration is preserved over a firmware reload, and
what is lost?
Can i add VLAN in duel MAC mode and then swap into the switch firmware
and all the VLANs are preserved? Can i add fdb entries to a port in
dual MAC mode, and then swap into the swtich firmware and the FDB
table is preserved? What about STP port state? What about ... ?
> +bool prueth_dev_check(const struct net_device *ndev)
> +{
> + if (ndev->netdev_ops == &emac_netdev_ops && netif_running(ndev)) {
> + struct prueth_emac *emac = netdev_priv(ndev);
> +
> + return emac->prueth->is_switch_mode;
> + }
> +
> + return false;
> +}
This does not appear to be used anywhere?
Andrew
Hi Andrew,
On 27/03/24 6:05 pm, Andrew Lunn wrote:
> On Wed, Mar 27, 2024 at 05:10:54PM +0530, 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.
>
> Patch 0/3 does not say this. It just shows the interfaces being added
I will modify the cover letter to state that.
> to the bridge. There should not be any need to down the interfaces.
>
The interfaces needs to be turned down for switching between dual emac
and switch mode.
Dual Emac mode runs with ICSSG Dual Emac firmware where as Switch mode
works with ICSSG Switch firmware. These firmware are running on the
dedicated PRU RPROC cores (pru0, rtu0, txpru0). When switch mode is
enabled, these pru cores need to be stopped and then Switch firmware is
loaded on these cores and then the cores are started again.
We stop the cores when interfaces are down and start the cores when
interfaces are up.
In short, Dual EMAC firmware runs on pru cores, we put down the
interface, stop pru cores, load switch firmware on the cores, bring the
interface up and start the pru cores and now Switch mode is enabled.
>> 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
>
> I keep asking this, so it would be good to explain it in the commit
> message. What configuration is preserved over a firmware reload, and
> what is lost?
>
> Can i add VLAN in duel MAC mode and then swap into the switch firmware
> and all the VLANs are preserved? Can i add fdb entries to a port in
> dual MAC mode, and then swap into the swtich firmware and the FDB
> table is preserved? What about STP port state? What about ... ?
>
When ports are brought up (firmware reload) we do a full cleaning of all
the shared memories i.e. SMEM (shared RAM). [1]
Vlan table and FDB table are stored in SMEM so all the configuration
done to VLAN / FDB tables will be lost.
We don't clear DRAM. DRAM is used for sending r30 commands [see
emac_r30_cmd_init()], configure half duplex [see
icssg_config_half_duplex()] and configure link speed [see
icssg_config_set_speed()]. r30 commands are used to set port state (stp).
Now when the interfaces are brought up (firmware reload) r30 command is
reconfigured as a result any changes done to port state (stp) will be
lost. But the duplex and speed settings will be preserved.
To summarize,
VLAN table / FDB table and port states are lost during a firmware reload.
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/ti/icssg/icssg_prueth.c#L1321
>
>> +bool prueth_dev_check(const struct net_device *ndev)
>> +{
>> + if (ndev->netdev_ops == &emac_netdev_ops && netif_running(ndev)) {
>> + struct prueth_emac *emac = netdev_priv(ndev);
>> +
>> + return emac->prueth->is_switch_mode;
>> + }
>> +
>> + return false;
>> +}
>
> This does not appear to be used anywhere?
>
> Andrew
--
Thanks and Regards,
Danish
On Thu, Mar 28, 2024 at 11:39:33AM +0530, MD Danish Anwar wrote:
> Hi Andrew,
>
> On 27/03/24 6:05 pm, Andrew Lunn wrote:
> > On Wed, Mar 27, 2024 at 05:10:54PM +0530, 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.
> >
> > Patch 0/3 does not say this. It just shows the interfaces being added
>
> I will modify the cover letter to state that.
>
> > to the bridge. There should not be any need to down the interfaces.
> >
>
> The interfaces needs to be turned down for switching between dual emac
> and switch mode.
>
> Dual Emac mode runs with ICSSG Dual Emac firmware where as Switch mode
> works with ICSSG Switch firmware. These firmware are running on the
> dedicated PRU RPROC cores (pru0, rtu0, txpru0). When switch mode is
> enabled, these pru cores need to be stopped and then Switch firmware is
> loaded on these cores and then the cores are started again.
>
> We stop the cores when interfaces are down and start the cores when
> interfaces are up.
>
> In short, Dual EMAC firmware runs on pru cores, we put down the
> interface, stop pru cores, load switch firmware on the cores, bring the
> interface up and start the pru cores and now Switch mode is enabled.
This is not the Linux model. Try it, add an interface to a software
bridge. It does not care if it is admin up or down.
You need to hide this difference in your driver.
> > I keep asking this, so it would be good to explain it in the commit
> > message. What configuration is preserved over a firmware reload, and
> > what is lost?
> >
> > Can i add VLAN in duel MAC mode and then swap into the switch firmware
> > and all the VLANs are preserved? Can i add fdb entries to a port in
> > dual MAC mode, and then swap into the swtich firmware and the FDB
> > table is preserved? What about STP port state? What about ... ?
> >
>
> When ports are brought up (firmware reload) we do a full cleaning of all
> the shared memories i.e. SMEM (shared RAM). [1]
>
> Vlan table and FDB table are stored in SMEM so all the configuration
> done to VLAN / FDB tables will be lost.
>
> We don't clear DRAM. DRAM is used for sending r30 commands [see
> emac_r30_cmd_init()], configure half duplex [see
> icssg_config_half_duplex()] and configure link speed [see
> icssg_config_set_speed()]. r30 commands are used to set port state (stp).
>
> Now when the interfaces are brought up (firmware reload) r30 command is
> reconfigured as a result any changes done to port state (stp) will be
> lost. But the duplex and speed settings will be preserved.
>
> To summarize,
> VLAN table / FDB table and port states are lost during a firmware reload.
So you also need to work around this in your driver. I think it is
possible to get the network stack to enumerate the configuration. Take
a look at the Mellanox driver. If i remember it does something like
this, but i don't remember the details.
Andrew
Hi Andrew,
On 28/03/24 6:09 pm, Andrew Lunn wrote:
> On Thu, Mar 28, 2024 at 11:39:33AM +0530, MD Danish Anwar wrote:
>> Hi Andrew,
>>
>> On 27/03/24 6:05 pm, Andrew Lunn wrote:
>>> On Wed, Mar 27, 2024 at 05:10:54PM +0530, 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.
>>>
>>> Patch 0/3 does not say this. It just shows the interfaces being added
>>
>> I will modify the cover letter to state that.
>>
>>> to the bridge. There should not be any need to down the interfaces.
>>>
>>
>> The interfaces needs to be turned down for switching between dual emac
>> and switch mode.
>>
>> Dual Emac mode runs with ICSSG Dual Emac firmware where as Switch mode
>> works with ICSSG Switch firmware. These firmware are running on the
>> dedicated PRU RPROC cores (pru0, rtu0, txpru0). When switch mode is
>> enabled, these pru cores need to be stopped and then Switch firmware is
>> loaded on these cores and then the cores are started again.
>>
>> We stop the cores when interfaces are down and start the cores when
>> interfaces are up.
>>
>> In short, Dual EMAC firmware runs on pru cores, we put down the
>> interface, stop pru cores, load switch firmware on the cores, bring the
>> interface up and start the pru cores and now Switch mode is enabled.
>
> This is not the Linux model. Try it, add an interface to a software
> bridge. It does not care if it is admin up or down.
>
> You need to hide this difference in your driver.
>
I have been working on this and have found a way to change firmwares
without bringing the interfaces up / down.
By default the interfaces are in MAC mode and the ICSSG EMAC firmwares
are running on pru cores. To enable switch mode we will need to stop the
cores and reload them with the ICSSG Switch firmwares. We can do this
without bringing the interfaces up / down.
When first interface is added to bridge it will still run emac
firmwares. The moment second interface gets added to bridge, we will
disable the ports and stop the pru cores. Load the switch firmwares on
to the cores, start the cores and enable the ports. All of this is done
from the driver.
The user need not to bring the interfaces up / down. Loading / Reloading
of firmwares will be handled inside the driver only. But we do need to
stop the cores for changing firmwares. For stopping the cores we will
change the port state to disable by sending r30 command to firmware.
As we are not restarting the interfaces, the DRAM, SMEM and MSMC RAM
doesn't get cleared. As a result with this approach all configurations
will be saved.
Please let me know if this approach looks ok to you.
Below will be the commands to enable switch mode now,
ip link add name br0 type bridge
ip link set dev eth1 master br0
ip link set dev eth2 master br0 (At this point we will stop the
cores, reload switch firmware, start the cores)
ip link set dev br0 up
bridge vlan add dev br0 vid 1 pvid untagged
>>> I keep asking this, so it would be good to explain it in the commit
>>> message. What configuration is preserved over a firmware reload, and
>>> what is lost?
>>>
>>> Can i add VLAN in duel MAC mode and then swap into the switch firmware
>>> and all the VLANs are preserved? Can i add fdb entries to a port in
>>> dual MAC mode, and then swap into the swtich firmware and the FDB
>>> table is preserved? What about STP port state? What about ... ?
>>>
>>
>> When ports are brought up (firmware reload) we do a full cleaning of all
>> the shared memories i.e. SMEM (shared RAM). [1]
>>
>> Vlan table and FDB table are stored in SMEM so all the configuration
>> done to VLAN / FDB tables will be lost.
>>
>> We don't clear DRAM. DRAM is used for sending r30 commands [see
>> emac_r30_cmd_init()], configure half duplex [see
>> icssg_config_half_duplex()] and configure link speed [see
>> icssg_config_set_speed()]. r30 commands are used to set port state (stp).
>>
>> Now when the interfaces are brought up (firmware reload) r30 command is
>> reconfigured as a result any changes done to port state (stp) will be
>> lost. But the duplex and speed settings will be preserved.
>>
>> To summarize,
>> VLAN table / FDB table and port states are lost during a firmware reload.
>
> So you also need to work around this in your driver. I think it is
> possible to get the network stack to enumerate the configuration. Take
> a look at the Mellanox driver. If i remember it does something like
> this, but i don't remember the details.
>
> Andrew
--
Thanks and Regards,
Danish
> I have been working on this and have found a way to change firmwares
> without bringing the interfaces up / down.
>
> By default the interfaces are in MAC mode and the ICSSG EMAC firmwares
> are running on pru cores. To enable switch mode we will need to stop the
> cores and reload them with the ICSSG Switch firmwares. We can do this
> without bringing the interfaces up / down.
>
> When first interface is added to bridge it will still run emac
> firmwares. The moment second interface gets added to bridge, we will
> disable the ports and stop the pru cores. Load the switch firmwares on
> to the cores, start the cores and enable the ports. All of this is done
> from the driver.
>
> The user need not to bring the interfaces up / down. Loading / Reloading
> of firmwares will be handled inside the driver only. But we do need to
> stop the cores for changing firmwares. For stopping the cores we will
> change the port state to disable by sending r30 command to firmware.
>
> As we are not restarting the interfaces, the DRAM, SMEM and MSMC RAM
> doesn't get cleared. As a result with this approach all configurations
> will be saved.
>
> Please let me know if this approach looks ok to you.
>
> Below will be the commands to enable switch mode now,
>
> ip link add name br0 type bridge
> ip link set dev eth1 master br0
> ip link set dev eth2 master br0 (At this point we will stop the
> cores, reload switch firmware, start the cores)
> ip link set dev br0 up
> bridge vlan add dev br0 vid 1 pvid untagged
This sounds a lot better.
Note that the bridge interface br0 might already be up when the
interfaces are added. So that is a different order to what you showed
here.
There will be some packet losses when you swap firmware, but it
probably is not that bad. When interfaces are added to a bridge
packets are dropped anywhere while spanning tree determines the
network topology. It will just appear your device is slow at doing
that.
Andrew
On 4/10/2024 6:12 PM, Andrew Lunn wrote:
>> I have been working on this and have found a way to change firmwares
>> without bringing the interfaces up / down.
>>
>> By default the interfaces are in MAC mode and the ICSSG EMAC firmwares
>> are running on pru cores. To enable switch mode we will need to stop the
>> cores and reload them with the ICSSG Switch firmwares. We can do this
>> without bringing the interfaces up / down.
>>
>> When first interface is added to bridge it will still run emac
>> firmwares. The moment second interface gets added to bridge, we will
>> disable the ports and stop the pru cores. Load the switch firmwares on
>> to the cores, start the cores and enable the ports. All of this is done
>> from the driver.
>>
>> The user need not to bring the interfaces up / down. Loading / Reloading
>> of firmwares will be handled inside the driver only. But we do need to
>> stop the cores for changing firmwares. For stopping the cores we will
>> change the port state to disable by sending r30 command to firmware.
>>
>> As we are not restarting the interfaces, the DRAM, SMEM and MSMC RAM
>> doesn't get cleared. As a result with this approach all configurations
>> will be saved.
>>
>> Please let me know if this approach looks ok to you.
>>
>> Below will be the commands to enable switch mode now,
>>
>> ip link add name br0 type bridge
>> ip link set dev eth1 master br0
>> ip link set dev eth2 master br0 (At this point we will stop the
>> cores, reload switch firmware, start the cores)
>> ip link set dev br0 up
>> bridge vlan add dev br0 vid 1 pvid untagged
>
> This sounds a lot better.
>
> Note that the bridge interface br0 might already be up when the
> interfaces are added. So that is a different order to what you showed
> here.
>
Hi Andrew, I have tested with that sequence as well and forwarding
works. Even if the second interface is added to bridge after bridge is
up, the forwarding works fine.
> There will be some packet losses when you swap firmware, but it
> probably is not that bad. When interfaces are added to a bridge
> packets are dropped anywhere while spanning tree determines the
> network topology. It will just appear your device is slow at doing
> that.
>
Yes there will be packet losses for a slight amount of time but that's
something we can't avoid.
I will post the next revision with these changes soon. Thanks for the
review.
> Andrew
--
Thanks and Regards,
Md Danish Anwar
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..).
Andrew
> Signed-off-by: MD Danish Anwar <[email protected]>
> ---
> drivers/net/ethernet/ti/Kconfig | 1 +
> drivers/net/ethernet/ti/Makefile | 3 +-
> drivers/net/ethernet/ti/icssg/icssg_config.c | 136 +++++++++++--
> drivers/net/ethernet/ti/icssg/icssg_config.h | 7 +
> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 198 ++++++++++++++++++-
> 5 files changed, 332 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
> index 1530d13984d4..9b5f5f680b35 100644
> --- a/drivers/net/ethernet/ti/Kconfig
> +++ b/drivers/net/ethernet/ti/Kconfig
> @@ -188,6 +188,7 @@ config TI_ICSSG_PRUETH
> select TI_ICSS_IEP
> select TI_K3_CPPI_DESC_POOL
> depends on PRU_REMOTEPROC
> + depends on NET_SWITCHDEV
> depends on ARCH_K3 && OF && TI_K3_UDMA_GLUE_LAYER
> depends on PTP_1588_CLOCK_OPTIONAL
> help
> diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
> index d8590304f3df..d295bded7a32 100644
> --- a/drivers/net/ethernet/ti/Makefile
> +++ b/drivers/net/ethernet/ti/Makefile
> @@ -38,5 +38,6 @@ icssg-prueth-y := icssg/icssg_prueth.o \
> icssg/icssg_config.o \
> icssg/icssg_mii_cfg.o \
> icssg/icssg_stats.o \
> - icssg/icssg_ethtool.o
> + icssg/icssg_ethtool.o \
> + icssg/icssg_switchdev.o
> obj-$(CONFIG_TI_ICSS_IEP) += icssg/icss_iep.o
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
> index 970e6ef9ba64..5b2064d1b03b 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
> @@ -105,28 +105,49 @@ static const struct map hwq_map[2][ICSSG_NUM_OTHER_QUEUES] = {
> },
> };
>
> +static void icssg_config_mii_init_switch(struct prueth_emac *emac)
> +{
> + struct prueth *prueth = emac->prueth;
> + int mii = prueth_emac_slice(emac);
> + u32 txcfg_reg, pcnt_reg, txcfg;
> + struct regmap *mii_rt;
> +
> + mii_rt = prueth->mii_rt;
> +
> + txcfg_reg = (mii == ICSS_MII0) ? PRUSS_MII_RT_TXCFG0 :
> + PRUSS_MII_RT_TXCFG1;
> + pcnt_reg = (mii == ICSS_MII0) ? PRUSS_MII_RT_RX_PCNT0 :
> + PRUSS_MII_RT_RX_PCNT1;
> +
> + txcfg = PRUSS_MII_RT_TXCFG_TX_ENABLE |
> + PRUSS_MII_RT_TXCFG_TX_AUTO_PREAMBLE |
> + PRUSS_MII_RT_TXCFG_TX_IPG_WIRE_CLK_EN;
> +
> + if (emac->phy_if == PHY_INTERFACE_MODE_MII && mii == ICSS_MII1)
> + txcfg |= PRUSS_MII_RT_TXCFG_TX_MUX_SEL;
> + else if (emac->phy_if != PHY_INTERFACE_MODE_MII && mii == ICSS_MII0)
> + txcfg |= PRUSS_MII_RT_TXCFG_TX_MUX_SEL;
> +
> + regmap_write(mii_rt, txcfg_reg, txcfg);
> + regmap_write(mii_rt, pcnt_reg, 0x1);
> +}
> +
> static void icssg_config_mii_init(struct prueth_emac *emac)
> {
> - u32 rxcfg, txcfg, rxcfg_reg, txcfg_reg, pcnt_reg;
> struct prueth *prueth = emac->prueth;
> int slice = prueth_emac_slice(emac);
> + u32 txcfg, txcfg_reg, pcnt_reg;
> struct regmap *mii_rt;
>
> mii_rt = prueth->mii_rt;
>
> - rxcfg_reg = (slice == ICSS_MII0) ? PRUSS_MII_RT_RXCFG0 :
> - PRUSS_MII_RT_RXCFG1;
> txcfg_reg = (slice == ICSS_MII0) ? PRUSS_MII_RT_TXCFG0 :
> PRUSS_MII_RT_TXCFG1;
> pcnt_reg = (slice == ICSS_MII0) ? PRUSS_MII_RT_RX_PCNT0 :
> PRUSS_MII_RT_RX_PCNT1;
>
> - rxcfg = MII_RXCFG_DEFAULT;
> txcfg = MII_TXCFG_DEFAULT;
>
> - if (slice == ICSS_MII1)
> - rxcfg |= PRUSS_MII_RT_RXCFG_RX_MUX_SEL;
> -
> /* In MII mode TX lines swapped inside ICSSG, so TX_MUX_SEL cfg need
> * to be swapped also comparing to RGMII mode.
> */
> @@ -135,7 +156,6 @@ static void icssg_config_mii_init(struct prueth_emac *emac)
> else if (emac->phy_if != PHY_INTERFACE_MODE_MII && slice == ICSS_MII1)
> txcfg |= PRUSS_MII_RT_TXCFG_TX_MUX_SEL;
>
> - regmap_write(mii_rt, rxcfg_reg, rxcfg);
> regmap_write(mii_rt, txcfg_reg, txcfg);
> regmap_write(mii_rt, pcnt_reg, 0x1);
> }
> @@ -249,6 +269,66 @@ static int emac_r30_is_done(struct prueth_emac *emac)
> return 1;
> }
>
> +static int prueth_switch_buffer_setup(struct prueth_emac *emac)
> +{
> + struct icssg_buffer_pool_cfg __iomem *bpool_cfg;
> + struct icssg_rxq_ctx __iomem *rxq_ctx;
> + struct prueth *prueth = emac->prueth;
> + int slice = prueth_emac_slice(emac);
> + u32 addr;
> + int i;
> +
> + addr = lower_32_bits(prueth->msmcram.pa);
> + if (slice)
> + addr += PRUETH_NUM_BUF_POOLS * PRUETH_EMAC_BUF_POOL_SIZE;
> +
> + if (addr % SZ_64K) {
> + dev_warn(prueth->dev, "buffer pool needs to be 64KB aligned\n");
> + return -EINVAL;
> + }
> +
> + bpool_cfg = emac->dram.va + BUFFER_POOL_0_ADDR_OFFSET;
> + /* workaround for f/w bug. bpool 0 needs to be initilalized */
> + for (i = 0; i < PRUETH_NUM_BUF_POOLS; i++) {
> + writel(addr, &bpool_cfg[i].addr);
> + writel(PRUETH_EMAC_BUF_POOL_SIZE, &bpool_cfg[i].len);
> + addr += PRUETH_EMAC_BUF_POOL_SIZE;
> + }
> +
> + if (!slice)
> + addr += PRUETH_NUM_BUF_POOLS * PRUETH_EMAC_BUF_POOL_SIZE;
> + else
> + addr += PRUETH_SW_NUM_BUF_POOLS_HOST * PRUETH_SW_BUF_POOL_SIZE_HOST;
> +
> + for (i = PRUETH_NUM_BUF_POOLS;
> + i < 2 * PRUETH_SW_NUM_BUF_POOLS_HOST + PRUETH_NUM_BUF_POOLS;
> + i++) {
> + /* The driver only uses first 4 queues per PRU so only initialize them */
> + if (i % PRUETH_SW_NUM_BUF_POOLS_HOST < PRUETH_SW_NUM_BUF_POOLS_PER_PRU) {
> + writel(addr, &bpool_cfg[i].addr);
> + writel(PRUETH_SW_BUF_POOL_SIZE_HOST, &bpool_cfg[i].len);
> + addr += PRUETH_SW_BUF_POOL_SIZE_HOST;
> + } else {
> + writel(0, &bpool_cfg[i].addr);
> + writel(0, &bpool_cfg[i].len);
> + }
> + }
> +
> + if (!slice)
> + addr += PRUETH_SW_NUM_BUF_POOLS_HOST * PRUETH_SW_BUF_POOL_SIZE_HOST;
> + else
> + addr += PRUETH_EMAC_RX_CTX_BUF_SIZE;
> +
> + rxq_ctx = emac->dram.va + HOST_RX_Q_PRE_CONTEXT_OFFSET;
> + for (i = 0; i < 3; i++)
> + writel(addr, &rxq_ctx->start[i]);
> +
> + addr += PRUETH_EMAC_RX_CTX_BUF_SIZE;
> + writel(addr - SZ_2K, &rxq_ctx->end);
> +
> + return 0;
> +}
> +
> static int prueth_emac_buffer_setup(struct prueth_emac *emac)
> {
> struct icssg_buffer_pool_cfg __iomem *bpool_cfg;
> @@ -325,13 +405,41 @@ static void icssg_init_emac_mode(struct prueth *prueth)
> icssg_class_set_host_mac_addr(prueth->miig_rt, mac);
> }
>
> +static void icssg_init_switch_mode(struct prueth *prueth)
> +{
> + u32 addr = prueth->shram.pa + EMAC_ICSSG_SWITCH_DEFAULT_VLAN_TABLE_OFFSET;
> + int i;
> +
> + if (prueth->emacs_initialized)
> + return;
> +
> + /* Set VLAN TABLE address base */
> + regmap_update_bits(prueth->miig_rt, FDB_GEN_CFG1, SMEM_VLAN_OFFSET_MASK,
> + addr << SMEM_VLAN_OFFSET);
> + /* Set enable VLAN aware mode, and FDBs for all PRUs */
> + regmap_write(prueth->miig_rt, FDB_GEN_CFG2, FDB_EN_ALL);
> + prueth->vlan_tbl = (struct prueth_vlan_tbl __force *)(prueth->shram.va +
> + EMAC_ICSSG_SWITCH_DEFAULT_VLAN_TABLE_OFFSET);
> + for (i = 0; i < SZ_4K - 1; i++) {
> + prueth->vlan_tbl[i].fid = i;
> + prueth->vlan_tbl[i].fid_c1 = 0;
> + }
> +
> + if (prueth->hw_bridge_dev)
> + icssg_class_set_host_mac_addr(prueth->miig_rt, prueth->hw_bridge_dev->dev_addr);
> + icssg_set_pvid(prueth, prueth->default_vlan, PRUETH_PORT_HOST);
> +}
> +
> int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
> {
> void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
> struct icssg_flow_cfg __iomem *flow_cfg;
> int ret;
>
> - icssg_init_emac_mode(prueth);
> + if (prueth->is_switch_mode)
> + icssg_init_switch_mode(prueth);
> + else
> + icssg_init_emac_mode(prueth);
>
> memset_io(config, 0, TAS_GATE_MASK_LIST0);
> icssg_miig_queues_init(prueth, slice);
> @@ -345,7 +453,10 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
> regmap_update_bits(prueth->miig_rt, ICSSG_CFG_OFFSET,
> ICSSG_CFG_DEFAULT, ICSSG_CFG_DEFAULT);
> icssg_miig_set_interface_mode(prueth->miig_rt, slice, emac->phy_if);
> - icssg_config_mii_init(emac);
> + if (prueth->is_switch_mode)
> + icssg_config_mii_init_switch(emac);
> + else
> + icssg_config_mii_init(emac);
> icssg_config_ipg(emac);
> icssg_update_rgmii_cfg(prueth->miig_rt, emac);
>
> @@ -368,7 +479,10 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
> writeb(0, config + SPL_PKT_DEFAULT_PRIORITY);
> writeb(0, config + QUEUE_NUM_UNTAGGED);
>
> - ret = prueth_emac_buffer_setup(emac);
> + if (prueth->is_switch_mode)
> + ret = prueth_switch_buffer_setup(emac);
> + else
> + ret = prueth_emac_buffer_setup(emac);
> if (ret)
> return ret;
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
> index 0d5d5d253b7a..cc923f1d4387 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
> @@ -35,6 +35,13 @@ struct icssg_flow_cfg {
> (2 * (PRUETH_EMAC_BUF_POOL_SIZE * PRUETH_NUM_BUF_POOLS + \
> PRUETH_EMAC_RX_CTX_BUF_SIZE * 2))
>
> +#define PRUETH_SW_BUF_POOL_SIZE_HOST SZ_4K
> +#define PRUETH_SW_NUM_BUF_POOLS_HOST 8
> +#define PRUETH_SW_NUM_BUF_POOLS_PER_PRU 4
> +#define MSMC_RAM_SIZE_SWITCH_MODE \
> + (MSMC_RAM_SIZE + \
> + (2 * PRUETH_SW_BUF_POOL_SIZE_HOST * PRUETH_SW_NUM_BUF_POOLS_HOST))
> +
> #define PRUETH_SWITCH_FDB_MASK ((SIZE_OF_FDB / NUMBER_OF_FDB_BUCKET_ENTRIES) - 1)
>
> struct icssg_rxq_ctx {
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index 9168ab3c4b9e..725b5de05e00 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -27,6 +27,7 @@
> #include <linux/remoteproc/pruss.h>
> #include <linux/regmap.h>
> #include <linux/remoteproc.h>
> +#include <net/switchdev.h>
>
> #include "icssg_prueth.h"
> #include "icssg_mii_rt.h"
> @@ -54,6 +55,10 @@
>
> #define prueth_napi_to_emac(napi) container_of(napi, struct prueth_emac, napi_rx)
>
> +#define DEFAULT_VID 1
> +#define DEFAULT_PORT_MASK 1
> +#define DEFAULT_UNTAG_MASK 1
> +
> /* CTRLMMR_ICSSG_RGMII_CTRL register bits */
> #define ICSSG_CTRL_RGMII_ID_MODE BIT(24)
>
> @@ -558,6 +563,8 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
> } else {
> /* send the filled skb up the n/w stack */
> skb_put(skb, pkt_len);
> + if (emac->prueth->is_switch_mode)
> + skb->offload_fwd_mark = emac->offload_fwd_mark;
> skb->protocol = eth_type_trans(skb, ndev);
> napi_gro_receive(&emac->napi_rx, skb);
> ndev->stats.rx_bytes += pkt_len;
> @@ -890,6 +897,19 @@ struct icssg_firmwares {
> char *txpru;
> };
>
> +static struct icssg_firmwares icssg_switch_firmwares[] = {
> + {
> + .pru = "ti-pruss/am65x-sr2-pru0-prusw-fw.elf",
> + .rtu = "ti-pruss/am65x-sr2-rtu0-prusw-fw.elf",
> + .txpru = "ti-pruss/am65x-sr2-txpru0-prusw-fw.elf",
> + },
> + {
> + .pru = "ti-pruss/am65x-sr2-pru1-prusw-fw.elf",
> + .rtu = "ti-pruss/am65x-sr2-rtu1-prusw-fw.elf",
> + .txpru = "ti-pruss/am65x-sr2-txpru1-prusw-fw.elf",
> + }
> +};
> +
> static struct icssg_firmwares icssg_emac_firmwares[] = {
> {
> .pru = "ti-pruss/am65x-sr2-pru0-prueth-fw.elf",
> @@ -909,7 +929,10 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> struct device *dev = prueth->dev;
> int slice, ret;
>
> - firmwares = icssg_emac_firmwares;
> + if (prueth->is_switch_mode)
> + firmwares = icssg_switch_firmwares;
> + else
> + firmwares = icssg_emac_firmwares;
>
> slice = prueth_emac_slice(emac);
> if (slice < 0) {
> @@ -1411,6 +1434,21 @@ static int emac_ndo_open(struct net_device *ndev)
>
> queue_work(system_long_wq, &emac->stats_work.work);
>
> + if (prueth->is_switch_mode) {
> + icssg_fdb_add_del(emac, eth_stp_addr, prueth->default_vlan,
> + ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
> + ICSSG_FDB_ENTRY_P1_MEMBERSHIP |
> + ICSSG_FDB_ENTRY_P2_MEMBERSHIP |
> + ICSSG_FDB_ENTRY_BLOCK,
> + true);
> + icssg_vtbl_modify(emac, emac->port_vlan | DEFAULT_VID,
> + BIT(emac->port_id) | DEFAULT_PORT_MASK,
> + BIT(emac->port_id) | DEFAULT_UNTAG_MASK,
> + true);
> + icssg_set_pvid(emac->prueth, emac->port_vlan, emac->port_id);
> + emac_set_port_state(emac, ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE);
> + }
> +
> return 0;
>
> reset_tx_chan:
> @@ -1945,6 +1983,148 @@ static void prueth_put_cores(struct prueth *prueth, int slice)
> pru_rproc_put(prueth->pru[slice]);
> }
>
> +static void prueth_offload_fwd_mark_update(struct prueth *prueth)
> +{
> + int set_val = 0;
> + int i;
> +
> + if (prueth->br_members == (PRUETH_PORT_MII0 | PRUETH_PORT_MII1))
> + set_val = 1;
> +
> + dev_dbg(prueth->dev, "set offload_fwd_mark %d\n", set_val);
> +
> + for (i = PRUETH_MAC0; i < PRUETH_NUM_MACS; i++) {
> + struct prueth_emac *emac = prueth->emac[i];
> +
> + if (!emac || !emac->ndev)
> + continue;
> +
> + emac->offload_fwd_mark = set_val;
> + }
> +}
> +
> +bool prueth_dev_check(const struct net_device *ndev)
> +{
> + if (ndev->netdev_ops == &emac_netdev_ops && netif_running(ndev)) {
> + struct prueth_emac *emac = netdev_priv(ndev);
> +
> + return emac->prueth->is_switch_mode;
> + }
> +
> + return false;
> +}
> +
> +static int prueth_netdevice_port_link(struct net_device *ndev,
> + struct net_device *br_ndev,
> + struct netlink_ext_ack *extack)
> +{
> + struct prueth_emac *emac = netdev_priv(ndev);
> + struct prueth *prueth = emac->prueth;
> + int err;
> +
> + if (!prueth->br_members) {
> + prueth->hw_bridge_dev = br_ndev;
> + } else {
> + /* This is adding the port to a second bridge, this is
> + * unsupported
> + */
> + if (prueth->hw_bridge_dev != br_ndev)
> + return -EOPNOTSUPP;
> + }
> +
> + err = switchdev_bridge_port_offload(br_ndev, ndev, emac,
> + &prueth->prueth_switchdev_nb,
> + &prueth->prueth_switchdev_bl_nb,
> + false, extack);
> + if (err)
> + return err;
> +
> + prueth->br_members |= BIT(emac->port_id);
> +
> + if (prueth->br_members & BIT(PRUETH_PORT_MII0) &&
> + prueth->br_members & BIT(PRUETH_PORT_MII1)) {
> + prueth->is_switch_mode = true;
> + prueth->default_vlan = 1;
> + emac->port_vlan = prueth->default_vlan;
> + }
> +
> + prueth_offload_fwd_mark_update(prueth);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static void prueth_netdevice_port_unlink(struct net_device *ndev)
> +{
> + struct prueth_emac *emac = netdev_priv(ndev);
> + struct prueth *prueth = emac->prueth;
> +
> + prueth->br_members &= ~BIT(emac->port_id);
> +
> + prueth->is_switch_mode = false;
> + emac->port_vlan = 0;
> +
> + prueth_offload_fwd_mark_update(prueth);
> +
> + if (!prueth->br_members)
> + prueth->hw_bridge_dev = NULL;
> +}
> +
> +/* netdev notifier */
> +static int prueth_netdevice_event(struct notifier_block *unused,
> + unsigned long event, void *ptr)
> +{
> + struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(ptr);
> + struct net_device *ndev = netdev_notifier_info_to_dev(ptr);
> + struct netdev_notifier_changeupper_info *info;
> + int ret = NOTIFY_DONE;
> +
> + if (ndev->netdev_ops != &emac_netdev_ops)
> + return NOTIFY_DONE;
> +
> + switch (event) {
> + case NETDEV_CHANGEUPPER:
> + info = ptr;
> +
> + if (netif_is_bridge_master(info->upper_dev)) {
> + if (info->linking)
> + ret = prueth_netdevice_port_link(ndev, info->upper_dev, extack);
> + else
> + prueth_netdevice_port_unlink(ndev);
> + }
> + break;
> + default:
> + return NOTIFY_DONE;
> + }
> +
> + return notifier_from_errno(ret);
> +}
> +
> +static int prueth_register_notifiers(struct prueth *prueth)
> +{
> + int ret = 0;
> +
> + prueth->prueth_netdevice_nb.notifier_call = &prueth_netdevice_event;
> + ret = register_netdevice_notifier(&prueth->prueth_netdevice_nb);
> + if (ret) {
> + dev_err(prueth->dev, "can't register netdevice notifier\n");
> + return ret;
> + }
> +
> + ret = prueth_switchdev_register_notifiers(prueth);
> + if (ret)
> + unregister_netdevice_notifier(&prueth->prueth_netdevice_nb);
> +
> + return ret;
> +}
> +
> +static void prueth_unregister_notifiers(struct prueth *prueth)
> +{
> + prueth_switchdev_unregister_notifiers(prueth);
> + unregister_netdevice_notifier(&prueth->prueth_netdevice_nb);
> +}
> +
> +static const struct of_device_id prueth_dt_match[];
> +
> static int prueth_probe(struct platform_device *pdev)
> {
> struct device_node *eth_node, *eth_ports_node;
> @@ -2072,6 +2252,10 @@ static int prueth_probe(struct platform_device *pdev)
> }
>
> msmc_ram_size = MSMC_RAM_SIZE;
> + prueth->is_switchmode_supported = prueth->pdata.switch_mode;
> + if (prueth->is_switchmode_supported)
> + msmc_ram_size = MSMC_RAM_SIZE_SWITCH_MODE;
> +
>
> /* NOTE: FW bug needs buffer base to be 64KB aligned */
> prueth->msmcram.va =
> @@ -2167,6 +2351,14 @@ static int prueth_probe(struct platform_device *pdev)
> phy_attached_info(prueth->emac[PRUETH_MAC1]->ndev->phydev);
> }
>
> + if (prueth->is_switchmode_supported) {
> + ret = prueth_register_notifiers(prueth);
> + if (ret)
> + goto netdev_unregister;
> +
> + sprintf(prueth->switch_id, "%s", dev_name(dev));
> + }
> +
> dev_info(dev, "TI PRU ethernet driver initialized: %s EMAC mode\n",
> (!eth0_node || !eth1_node) ? "single" : "dual");
>
> @@ -2236,6 +2428,8 @@ static void prueth_remove(struct platform_device *pdev)
> struct device_node *eth_node;
> int i;
>
> + prueth_unregister_notifiers(prueth);
> +
> for (i = 0; i < PRUETH_NUM_MACS; i++) {
> if (!prueth->registered_netdevs[i])
> continue;
> @@ -2333,10 +2527,12 @@ static const struct dev_pm_ops prueth_dev_pm_ops = {
> static const struct prueth_pdata am654_icssg_pdata = {
> .fdqring_mode = K3_RINGACC_RING_MODE_MESSAGE,
> .quirk_10m_link_issue = 1,
> + .switch_mode = 1,
> };
>
> 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[] = {
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
On 4/15/24 1:56 AM, Anwar, Md Danish wrote:
> 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.
>
Yes I saw that and understand all this, but that is not my question.
> 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.
>
Same, I see what we do here, this doesn't give me the "why".
> 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.
So this seems to almost answer the question, but I still do not see
why we could not use the ports separately when using SWITCH firmware.
Why not have a filter/rule set by default to each port so that they
do not forward packets automatically but instead always forward
to the host port? That would result in the same functionality as
the Dual-EMAC firmware, but without all the mess of runtime firmware
switching based on usecase (simply update the forwarding rules when
in bridge mode).
Andrew
>
> 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[] = {
>
On 16/04/24 10:49 pm, Andrew Davis wrote:
> On 4/15/24 1:56 AM, Anwar, Md Danish wrote:
>> 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.
>>
>
> Yes I saw that and understand all this, but that is not my question.
>
>> 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.
>>
>
> Same, I see what we do here, this doesn't give me the "why".
>
>> 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.
>
> So this seems to almost answer the question, but I still do not see
> why we could not use the ports separately when using SWITCH firmware.
>
This was discussed during v1 of this series [1]. Andrew Lunn also
commented asking for a single firmware that can do both. But the problem
is having a single firmware to do both Switch and EMAC operation will
require additional check in firmware to check whether we are in switch
or mac mode based on that firmware will forward the packet to the other
port or host. I had talked to firmware team regarding this, this
additional check will result in consuming alot of extra cycles and the
performance will be degraded substantially. That is why we decided to
stick with two different firmwares as combining them in one has a very
big penalty.
Such firmware doesn't exist as of now. Furthermore, we are also working
on hsr mode for ICSSG driver and this will also introduce new hsr
firmware. In future their could be more different modes. Now combining
all this firmwares into one will consume all the budget cycles and that
is why firmware team has decided to have split firmware.
While switching modes, stopping and restarting the PRU cores after
changing firmware works fine and it's completely abstracted from user.
For a user it's same as a software bridge. There will be some packet
loss but as Andrew Lunn pointed out, even in sofware bridging there are
some packets losses.
> Why not have a filter/rule set by default to each port so that they
> do not forward packets automatically but instead always forward
> to the host port? That would result in the same functionality as
> the Dual-EMAC firmware, but without all the mess of runtime firmware
> switching based on usecase (simply update the forwarding rules when
> in bridge mode).
>
> Andrew
>
>>
>> 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[] = {
>>
[1] https://lore.kernel.org/all/[email protected]/
--
Thanks and Regards,
Danish