* [PATCH net v3 0/2] IEP clock module bug fixes
@ 2024-12-05 8:28 Meghana Malladi
2024-12-05 8:28 ` [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence Meghana Malladi
2024-12-05 8:28 ` [PATCH net v3 2/2] net: ti: icssg-prueth: Fix clearing of IEP_CMP_CFG registers during iep_init Meghana Malladi
0 siblings, 2 replies; 12+ messages in thread
From: Meghana Malladi @ 2024-12-05 8:28 UTC (permalink / raw)
To: vigneshr, jan.kiszka, Roger Quadros, m-malladi,
javier.carrasco.cruz, diogo.ivo, jacob.e.keller, horms, pabeni,
kuba, edumazet, davem, andrew+netdev
Cc: linux-kernel, netdev, linux-arm-kernel, srk, danishanwar
Hi All,
This series has some bug fixes for IEP module needed by PPS and
timesync operations.
Patch 1/2 fixes firmware load sequence to run all the firmwares
when either of the ethernet interfaces is up. Move all the code
common for firmware bringup under common functions.
Patch 2/2 fixes distorted PPS signal when the ethernet interfaces
are brough down and up. This patch also fixes enabling PPS signal
after bringing the interface up, without disabling PPS.
MD Danish Anwar (1):
net: ti: icssg-prueth: Fix firmware load sequence.
Meghana Malladi (1):
net: ti: icssg-prueth: Fix clearing of IEP_CMP_CFG registers during
iep_init
drivers/net/ethernet/ti/icssg/icss_iep.c | 9 ++
drivers/net/ethernet/ti/icssg/icssg_config.c | 45 ++++--
drivers/net/ethernet/ti/icssg/icssg_config.h | 1 +
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 157 ++++++++++++-------
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 5 +
5 files changed, 149 insertions(+), 68 deletions(-)
base-commit: dfc14664794a4706e0c2186a0c082386e6b14c4d
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence.
2024-12-05 8:28 [PATCH net v3 0/2] IEP clock module bug fixes Meghana Malladi
@ 2024-12-05 8:28 ` Meghana Malladi
2024-12-05 9:10 ` Kalesh Anakkur Purayil
2024-12-05 13:08 ` Roger Quadros
2024-12-05 8:28 ` [PATCH net v3 2/2] net: ti: icssg-prueth: Fix clearing of IEP_CMP_CFG registers during iep_init Meghana Malladi
1 sibling, 2 replies; 12+ messages in thread
From: Meghana Malladi @ 2024-12-05 8:28 UTC (permalink / raw)
To: vigneshr, jan.kiszka, Roger Quadros, m-malladi,
javier.carrasco.cruz, diogo.ivo, jacob.e.keller, horms, pabeni,
kuba, edumazet, davem, andrew+netdev
Cc: linux-kernel, netdev, linux-arm-kernel, srk, danishanwar
From: MD Danish Anwar <danishanwar@ti.com>
Timesync related operations are ran in PRU0 cores for both ICSSG SLICE0
and SLICE1. Currently whenever any ICSSG interface comes up we load the
respective firmwares to PRU cores and whenever interface goes down, we
stop the resective cores. Due to this, when SLICE0 goes down while
SLICE1 is still active, PRU0 firmwares are unloaded and PRU0 core is
stopped. This results in clock jump for SLICE1 interface as the timesync
related operations are no longer running.
As there are interdependencies between SLICE0 and SLICE1 firmwares,
fix this by running both PRU0 and PRU1 firmwares as long as at least 1
ICSSG interface is up. Add new flag in prueth struct to check if all
firmwares are running.
Use emacs_initialized as reference count to load the firmwares for the
first and last interface up/down. Moving init_emac_mode and fw_offload_mode
API outside of icssg_config to icssg_common_start API as they need
to be called only once per firmware boot.
Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---
Hi all,
This patch is based on net-next tagged next-20241128.
v2:https://lore.kernel.org/all/20241128122931.2494446-2-m-malladi@ti.com/
* Changes since v2 (v3-v2):
- error handling in caller function of prueth_emac_common_start()
- Use prus_running flag check before stopping the firmwares
Both suggested by Roger Quadros <rogerq@kernel.org>
drivers/net/ethernet/ti/icssg/icssg_config.c | 45 ++++--
drivers/net/ethernet/ti/icssg/icssg_config.h | 1 +
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 157 ++++++++++++-------
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 5 +
4 files changed, 140 insertions(+), 68 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
index 5d2491c2943a..342150756cf7 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
@@ -397,7 +397,7 @@ static int prueth_emac_buffer_setup(struct prueth_emac *emac)
return 0;
}
-static void icssg_init_emac_mode(struct prueth *prueth)
+void icssg_init_emac_mode(struct prueth *prueth)
{
/* When the device is configured as a bridge and it is being brought
* back to the emac mode, the host mac address has to be set as 0.
@@ -406,9 +406,6 @@ static void icssg_init_emac_mode(struct prueth *prueth)
int i;
u8 mac[ETH_ALEN] = { 0 };
- 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);
@@ -423,15 +420,13 @@ static void icssg_init_emac_mode(struct prueth *prueth)
/* Clear host MAC address */
icssg_class_set_host_mac_addr(prueth->miig_rt, mac);
}
+EXPORT_SYMBOL_GPL(icssg_init_emac_mode);
-static void icssg_init_fw_offload_mode(struct prueth *prueth)
+void icssg_init_fw_offload_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);
@@ -448,6 +443,7 @@ static void icssg_init_fw_offload_mode(struct prueth *prueth)
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);
}
+EXPORT_SYMBOL_GPL(icssg_init_fw_offload_mode);
int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
{
@@ -455,11 +451,6 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
struct icssg_flow_cfg __iomem *flow_cfg;
int ret;
- if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
- icssg_init_fw_offload_mode(prueth);
- else
- icssg_init_emac_mode(prueth);
-
memset_io(config, 0, TAS_GATE_MASK_LIST0);
icssg_miig_queues_init(prueth, slice);
@@ -786,3 +777,31 @@ void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port)
writel(pvid, prueth->shram.va + EMAC_ICSSG_SWITCH_PORT0_DEFAULT_VLAN_OFFSET);
}
EXPORT_SYMBOL_GPL(icssg_set_pvid);
+
+int emac_fdb_flow_id_updated(struct prueth_emac *emac)
+{
+ struct mgmt_cmd_rsp fdb_cmd_rsp = { 0 };
+ int slice = prueth_emac_slice(emac);
+ struct mgmt_cmd fdb_cmd = { 0 };
+ int ret = 0;
+
+ fdb_cmd.header = ICSSG_FW_MGMT_CMD_HEADER;
+ fdb_cmd.type = ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW;
+ fdb_cmd.seqnum = ++(emac->prueth->icssg_hwcmdseq);
+ fdb_cmd.param = 0;
+
+ fdb_cmd.param |= (slice << 4);
+ fdb_cmd.cmd_args[0] = 0;
+
+ ret = icssg_send_fdb_msg(emac, &fdb_cmd, &fdb_cmd_rsp);
+
+ if (ret)
+ return ret;
+
+ WARN_ON(fdb_cmd.seqnum != fdb_cmd_rsp.seqnum);
+ if (fdb_cmd_rsp.status == 1)
+ return 0;
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(emac_fdb_flow_id_updated);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
index 92c2deaa3068..c884e9fa099e 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
@@ -55,6 +55,7 @@ struct icssg_rxq_ctx {
#define ICSSG_FW_MGMT_FDB_CMD_TYPE 0x03
#define ICSSG_FW_MGMT_CMD_TYPE 0x04
#define ICSSG_FW_MGMT_PKT 0x80000000
+#define ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW 0x05
struct icssg_r30_cmd {
u32 cmd[4];
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index c568c84a032b..2e22e793b01a 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -164,11 +164,11 @@ static struct icssg_firmwares icssg_emac_firmwares[] = {
}
};
-static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
+static int prueth_emac_start(struct prueth *prueth, int slice)
{
struct icssg_firmwares *firmwares;
struct device *dev = prueth->dev;
- int slice, ret;
+ int ret;
if (prueth->is_switch_mode)
firmwares = icssg_switch_firmwares;
@@ -177,16 +177,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
else
firmwares = icssg_emac_firmwares;
- slice = prueth_emac_slice(emac);
- if (slice < 0) {
- netdev_err(emac->ndev, "invalid port\n");
- return -EINVAL;
- }
-
- ret = icssg_config(prueth, emac, slice);
- if (ret)
- return ret;
-
ret = rproc_set_firmware(prueth->pru[slice], firmwares[slice].pru);
ret = rproc_boot(prueth->pru[slice]);
if (ret) {
@@ -208,7 +198,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
goto halt_rtu;
}
- emac->fw_running = 1;
return 0;
halt_rtu:
@@ -220,6 +209,80 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
return ret;
}
+static int prueth_emac_common_start(struct prueth *prueth)
+{
+ struct prueth_emac *emac;
+ int ret = 0;
+ int slice;
+
+ if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
+ return -EINVAL;
+
+ /* clear SMEM and MSMC settings for all slices */
+ memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
+ memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
+
+ icssg_class_default(prueth->miig_rt, ICSS_SLICE0, 0, false);
+ icssg_class_default(prueth->miig_rt, ICSS_SLICE1, 0, false);
+
+ if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
+ icssg_init_fw_offload_mode(prueth);
+ else
+ icssg_init_emac_mode(prueth);
+
+ for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
+ emac = prueth->emac[slice];
+ if (emac) {
+ ret |= icssg_config(prueth, emac, slice);
+ if (ret)
+ return ret;
+ }
+ ret |= prueth_emac_start(prueth, slice);
+ }
+ if (!ret)
+ prueth->prus_running = 1;
+ else
+ return ret;
+
+ emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
+ prueth->emac[ICSS_SLICE1];
+ ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
+ emac, IEP_DEFAULT_CYCLE_TIME_NS);
+ if (ret) {
+ dev_err(prueth->dev, "Failed to initialize IEP module\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int prueth_emac_common_stop(struct prueth *prueth)
+{
+ struct prueth_emac *emac;
+ int slice;
+
+ if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
+ return -EINVAL;
+
+ icssg_class_disable(prueth->miig_rt, ICSS_SLICE0);
+ icssg_class_disable(prueth->miig_rt, ICSS_SLICE1);
+
+ for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
+ if (prueth->prus_running) {
+ rproc_shutdown(prueth->txpru[slice]);
+ rproc_shutdown(prueth->rtu[slice]);
+ rproc_shutdown(prueth->pru[slice]);
+ }
+ }
+ prueth->prus_running = 0;
+
+ emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
+ prueth->emac[ICSS_SLICE1];
+ icss_iep_exit(emac->iep);
+
+ return 0;
+}
+
/* called back by PHY layer if there is change in link state of hw port*/
static void emac_adjust_link(struct net_device *ndev)
{
@@ -369,12 +432,13 @@ static void prueth_iep_settime(void *clockops_data, u64 ns)
{
struct icssg_setclock_desc __iomem *sc_descp;
struct prueth_emac *emac = clockops_data;
+ struct prueth *prueth = emac->prueth;
struct icssg_setclock_desc sc_desc;
u64 cyclecount;
u32 cycletime;
int timeout;
- if (!emac->fw_running)
+ if (!prueth->prus_running)
return;
sc_descp = emac->prueth->shram.va + TIMESYNC_FW_WC_SETCLOCK_DESC_OFFSET;
@@ -543,23 +607,17 @@ static int emac_ndo_open(struct net_device *ndev)
{
struct prueth_emac *emac = netdev_priv(ndev);
int ret, i, num_data_chn = emac->tx_ch_num;
+ struct icssg_flow_cfg __iomem *flow_cfg;
struct prueth *prueth = emac->prueth;
int slice = prueth_emac_slice(emac);
struct device *dev = prueth->dev;
int max_rx_flows;
int rx_flow;
- /* clear SMEM and MSMC settings for all slices */
- if (!prueth->emacs_initialized) {
- memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
- memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
- }
-
/* set h/w MAC as user might have re-configured */
ether_addr_copy(emac->mac_addr, ndev->dev_addr);
icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
- icssg_class_default(prueth->miig_rt, slice, 0, false);
icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
/* Notify the stack of the actual queue counts. */
@@ -597,18 +655,23 @@ static int emac_ndo_open(struct net_device *ndev)
goto cleanup_napi;
}
- /* reset and start PRU firmware */
- ret = prueth_emac_start(prueth, emac);
- if (ret)
- goto free_rx_irq;
+ if (!prueth->emacs_initialized) {
+ ret = prueth_emac_common_start(prueth);
+ if (ret)
+ goto stop;
+ }
- icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
+ flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
+ writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);
+ ret = emac_fdb_flow_id_updated(emac);
- if (!prueth->emacs_initialized) {
- ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
- emac, IEP_DEFAULT_CYCLE_TIME_NS);
+ if (ret) {
+ netdev_err(ndev, "Failed to update Rx Flow ID %d", ret);
+ goto stop;
}
+ icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
+
ret = request_threaded_irq(emac->tx_ts_irq, NULL, prueth_tx_ts_irq,
IRQF_ONESHOT, dev_name(dev), emac);
if (ret)
@@ -653,8 +716,7 @@ static int emac_ndo_open(struct net_device *ndev)
free_tx_ts_irq:
free_irq(emac->tx_ts_irq, emac);
stop:
- prueth_emac_stop(emac);
-free_rx_irq:
+ prueth_emac_common_stop(prueth);
free_irq(emac->rx_chns.irq[rx_flow], emac);
cleanup_napi:
prueth_ndev_del_tx_napi(emac, emac->tx_ch_num);
@@ -689,8 +751,6 @@ static int emac_ndo_stop(struct net_device *ndev)
if (ndev->phydev)
phy_stop(ndev->phydev);
- icssg_class_disable(prueth->miig_rt, prueth_emac_slice(emac));
-
if (emac->prueth->is_hsr_offload_mode)
__dev_mc_unsync(ndev, icssg_prueth_hsr_del_mcast);
else
@@ -728,11 +788,9 @@ static int emac_ndo_stop(struct net_device *ndev)
/* Destroying the queued work in ndo_stop() */
cancel_delayed_work_sync(&emac->stats_work);
- if (prueth->emacs_initialized == 1)
- icss_iep_exit(emac->iep);
-
/* stop PRUs */
- prueth_emac_stop(emac);
+ if (prueth->emacs_initialized == 1)
+ prueth_emac_common_stop(prueth);
free_irq(emac->tx_ts_irq, emac);
@@ -1069,16 +1127,10 @@ static void prueth_emac_restart(struct prueth *prueth)
icssg_set_port_state(emac1, ICSSG_EMAC_PORT_DISABLE);
/* Stop both pru cores for both PRUeth ports*/
- prueth_emac_stop(emac0);
- prueth->emacs_initialized--;
- prueth_emac_stop(emac1);
- prueth->emacs_initialized--;
+ prueth_emac_common_stop(prueth);
/* Start both pru cores for both PRUeth ports */
- prueth_emac_start(prueth, emac0);
- prueth->emacs_initialized++;
- prueth_emac_start(prueth, emac1);
- prueth->emacs_initialized++;
+ prueth_emac_common_start(prueth);
/* Enable forwarding for both PRUeth ports */
icssg_set_port_state(emac0, ICSSG_EMAC_PORT_FORWARD);
@@ -1413,13 +1465,10 @@ static int prueth_probe(struct platform_device *pdev)
prueth->pa_stats = NULL;
}
- if (eth0_node) {
+ if (eth0_node || eth1_node) {
ret = prueth_get_cores(prueth, ICSS_SLICE0, false);
if (ret)
goto put_cores;
- }
-
- if (eth1_node) {
ret = prueth_get_cores(prueth, ICSS_SLICE1, false);
if (ret)
goto put_cores;
@@ -1618,14 +1667,12 @@ static int prueth_probe(struct platform_device *pdev)
pruss_put(prueth->pruss);
put_cores:
- if (eth1_node) {
- prueth_put_cores(prueth, ICSS_SLICE1);
- of_node_put(eth1_node);
- }
-
- if (eth0_node) {
+ if (eth0_node || eth1_node) {
prueth_put_cores(prueth, ICSS_SLICE0);
of_node_put(eth0_node);
+
+ prueth_put_cores(prueth, ICSS_SLICE1);
+ of_node_put(eth1_node);
}
return ret;
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index f5c1d473e9f9..b30f2e9a73d8 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -257,6 +257,7 @@ struct icssg_firmwares {
* @is_switchmode_supported: indicates platform support for switch mode
* @switch_id: ID for mapping switch ports to bridge
* @default_vlan: Default VLAN for host
+ * @prus_running: flag to indicate if all pru cores are running
*/
struct prueth {
struct device *dev;
@@ -298,6 +299,7 @@ struct prueth {
int default_vlan;
/** @vtbl_lock: Lock for vtbl in shared memory */
spinlock_t vtbl_lock;
+ bool prus_running;
};
struct emac_tx_ts_response {
@@ -361,6 +363,8 @@ int icssg_set_port_state(struct prueth_emac *emac,
enum icssg_port_state_cmd state);
void icssg_config_set_speed(struct prueth_emac *emac);
void icssg_config_half_duplex(struct prueth_emac *emac);
+void icssg_init_emac_mode(struct prueth *prueth);
+void icssg_init_fw_offload_mode(struct prueth *prueth);
/* Buffer queue helpers */
int icssg_queue_pop(struct prueth *prueth, u8 queue);
@@ -377,6 +381,7 @@ void icssg_vtbl_modify(struct prueth_emac *emac, u8 vid, u8 port_mask,
u8 untag_mask, bool add);
u16 icssg_get_pvid(struct prueth_emac *emac);
void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port);
+int emac_fdb_flow_id_updated(struct prueth_emac *emac);
#define prueth_napi_to_tx_chn(pnapi) \
container_of(pnapi, struct prueth_tx_chn, napi_tx)
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net v3 2/2] net: ti: icssg-prueth: Fix clearing of IEP_CMP_CFG registers during iep_init
2024-12-05 8:28 [PATCH net v3 0/2] IEP clock module bug fixes Meghana Malladi
2024-12-05 8:28 ` [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence Meghana Malladi
@ 2024-12-05 8:28 ` Meghana Malladi
1 sibling, 0 replies; 12+ messages in thread
From: Meghana Malladi @ 2024-12-05 8:28 UTC (permalink / raw)
To: vigneshr, jan.kiszka, Roger Quadros, m-malladi,
javier.carrasco.cruz, diogo.ivo, jacob.e.keller, horms, pabeni,
kuba, edumazet, davem, andrew+netdev
Cc: linux-kernel, netdev, linux-arm-kernel, srk, danishanwar
When ICSSG interfaces are brought down and brought up again, the
pru cores are shut down and booted again, flushing out all the memories
and start again in a clean state. Hence it is expected that the
IEP_CMP_CFG register needs to be flushed during iep_init() to ensure
that the existing residual configuration doesn't cause any unusual
behavior. If the register is not cleared, existing IEP_CMP_CFG set for
CMP1 will result in SYNC0_OUT signal based on the SYNC_OUT register values.
After bringing the interface up, calling PPS enable doesn't work as
the driver believes PPS is already enabled, (iep->pps_enabled is not
cleared during interface bring down) and driver will just return true
even though there is no signal. Fix this by disabling pps and perout.
Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
Reviewed-by: Roger Quadros <rogerq@kernel.org>
---
Hi all,
This patch is based on net-next tagged next-20241128
v2: https://lore.kernel.org/all/20241128122931.2494446-3-m-malladi@ti.com/
* Changes since v2 (v3-v2):
- Collected Reviewed-by tag from Roger Quadros <rogerq@kernel.org>
drivers/net/ethernet/ti/icssg/icss_iep.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.c b/drivers/net/ethernet/ti/icssg/icss_iep.c
index 5d6d1cf78e93..a96861debbe3 100644
--- a/drivers/net/ethernet/ti/icssg/icss_iep.c
+++ b/drivers/net/ethernet/ti/icssg/icss_iep.c
@@ -215,6 +215,10 @@ static void icss_iep_enable_shadow_mode(struct icss_iep *iep)
for (cmp = IEP_MIN_CMP; cmp < IEP_MAX_CMP; cmp++) {
regmap_update_bits(iep->map, ICSS_IEP_CMP_STAT_REG,
IEP_CMP_STATUS(cmp), IEP_CMP_STATUS(cmp));
+
+ regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
+ IEP_CMP_CFG_CMP_EN(cmp), 0);
+
}
/* enable reset counter on CMP0 event */
@@ -780,6 +784,11 @@ int icss_iep_exit(struct icss_iep *iep)
}
icss_iep_disable(iep);
+ if (iep->pps_enabled)
+ icss_iep_pps_enable(iep, false);
+ else if (iep->perout_enabled)
+ icss_iep_perout_enable(iep, NULL, false);
+
return 0;
}
EXPORT_SYMBOL_GPL(icss_iep_exit);
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence.
2024-12-05 8:28 ` [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence Meghana Malladi
@ 2024-12-05 9:10 ` Kalesh Anakkur Purayil
2024-12-09 10:39 ` [EXTERNAL] " Meghana Malladi
2024-12-05 13:08 ` Roger Quadros
1 sibling, 1 reply; 12+ messages in thread
From: Kalesh Anakkur Purayil @ 2024-12-05 9:10 UTC (permalink / raw)
To: Meghana Malladi
Cc: vigneshr, jan.kiszka, Roger Quadros, javier.carrasco.cruz,
diogo.ivo, jacob.e.keller, horms, pabeni, kuba, edumazet, davem,
andrew+netdev, linux-kernel, netdev, linux-arm-kernel, srk,
danishanwar
[-- Attachment #1: Type: text/plain, Size: 19340 bytes --]
On Thu, Dec 5, 2024 at 1:59 PM Meghana Malladi <m-malladi@ti.com> wrote:
>
> From: MD Danish Anwar <danishanwar@ti.com>
>
> Timesync related operations are ran in PRU0 cores for both ICSSG SLICE0
> and SLICE1. Currently whenever any ICSSG interface comes up we load the
> respective firmwares to PRU cores and whenever interface goes down, we
> stop the resective cores. Due to this, when SLICE0 goes down while
> SLICE1 is still active, PRU0 firmwares are unloaded and PRU0 core is
> stopped. This results in clock jump for SLICE1 interface as the timesync
> related operations are no longer running.
>
> As there are interdependencies between SLICE0 and SLICE1 firmwares,
> fix this by running both PRU0 and PRU1 firmwares as long as at least 1
> ICSSG interface is up. Add new flag in prueth struct to check if all
> firmwares are running.
>
> Use emacs_initialized as reference count to load the firmwares for the
> first and last interface up/down. Moving init_emac_mode and fw_offload_mode
> API outside of icssg_config to icssg_common_start API as they need
> to be called only once per firmware boot.
>
> Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
> ---
>
> Hi all,
>
> This patch is based on net-next tagged next-20241128.
> v2:https://lore.kernel.org/all/20241128122931.2494446-2-m-malladi@ti.com/
>
> * Changes since v2 (v3-v2):
> - error handling in caller function of prueth_emac_common_start()
> - Use prus_running flag check before stopping the firmwares
> Both suggested by Roger Quadros <rogerq@kernel.org>
>
> drivers/net/ethernet/ti/icssg/icssg_config.c | 45 ++++--
> drivers/net/ethernet/ti/icssg/icssg_config.h | 1 +
> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 157 ++++++++++++-------
> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 5 +
> 4 files changed, 140 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
> index 5d2491c2943a..342150756cf7 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
> @@ -397,7 +397,7 @@ static int prueth_emac_buffer_setup(struct prueth_emac *emac)
> return 0;
> }
>
> -static void icssg_init_emac_mode(struct prueth *prueth)
> +void icssg_init_emac_mode(struct prueth *prueth)
> {
> /* When the device is configured as a bridge and it is being brought
> * back to the emac mode, the host mac address has to be set as 0.
> @@ -406,9 +406,6 @@ static void icssg_init_emac_mode(struct prueth *prueth)
> int i;
> u8 mac[ETH_ALEN] = { 0 };
>
> - 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);
> @@ -423,15 +420,13 @@ static void icssg_init_emac_mode(struct prueth *prueth)
> /* Clear host MAC address */
> icssg_class_set_host_mac_addr(prueth->miig_rt, mac);
> }
> +EXPORT_SYMBOL_GPL(icssg_init_emac_mode);
>
> -static void icssg_init_fw_offload_mode(struct prueth *prueth)
> +void icssg_init_fw_offload_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);
> @@ -448,6 +443,7 @@ static void icssg_init_fw_offload_mode(struct prueth *prueth)
> 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);
> }
> +EXPORT_SYMBOL_GPL(icssg_init_fw_offload_mode);
>
> int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
> {
> @@ -455,11 +451,6 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
> struct icssg_flow_cfg __iomem *flow_cfg;
> int ret;
>
> - if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
> - icssg_init_fw_offload_mode(prueth);
> - else
> - icssg_init_emac_mode(prueth);
> -
> memset_io(config, 0, TAS_GATE_MASK_LIST0);
> icssg_miig_queues_init(prueth, slice);
>
> @@ -786,3 +777,31 @@ void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port)
> writel(pvid, prueth->shram.va + EMAC_ICSSG_SWITCH_PORT0_DEFAULT_VLAN_OFFSET);
> }
> EXPORT_SYMBOL_GPL(icssg_set_pvid);
> +
> +int emac_fdb_flow_id_updated(struct prueth_emac *emac)
> +{
> + struct mgmt_cmd_rsp fdb_cmd_rsp = { 0 };
> + int slice = prueth_emac_slice(emac);
> + struct mgmt_cmd fdb_cmd = { 0 };
> + int ret = 0;
[Kalesh] There is no need to initialize "ret" here
> +
> + fdb_cmd.header = ICSSG_FW_MGMT_CMD_HEADER;
> + fdb_cmd.type = ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW;
> + fdb_cmd.seqnum = ++(emac->prueth->icssg_hwcmdseq);
> + fdb_cmd.param = 0;
> +
> + fdb_cmd.param |= (slice << 4);
> + fdb_cmd.cmd_args[0] = 0;
> +
> + ret = icssg_send_fdb_msg(emac, &fdb_cmd, &fdb_cmd_rsp);
> +
[Kalesh] There is no need of an new line here
> + if (ret)
> + return ret;
> +
> + WARN_ON(fdb_cmd.seqnum != fdb_cmd_rsp.seqnum);
> + if (fdb_cmd_rsp.status == 1)
> + return 0;
> +
> + return -EINVAL;
[Kalesh] Maybe you can simplify this as:
return fdb_cmd_rsp.status == 1 ? 0 : -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(emac_fdb_flow_id_updated);
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
> index 92c2deaa3068..c884e9fa099e 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
> @@ -55,6 +55,7 @@ struct icssg_rxq_ctx {
> #define ICSSG_FW_MGMT_FDB_CMD_TYPE 0x03
> #define ICSSG_FW_MGMT_CMD_TYPE 0x04
> #define ICSSG_FW_MGMT_PKT 0x80000000
> +#define ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW 0x05
>
> struct icssg_r30_cmd {
> u32 cmd[4];
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index c568c84a032b..2e22e793b01a 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -164,11 +164,11 @@ static struct icssg_firmwares icssg_emac_firmwares[] = {
> }
> };
>
> -static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> +static int prueth_emac_start(struct prueth *prueth, int slice)
> {
> struct icssg_firmwares *firmwares;
> struct device *dev = prueth->dev;
> - int slice, ret;
> + int ret;
>
> if (prueth->is_switch_mode)
> firmwares = icssg_switch_firmwares;
> @@ -177,16 +177,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> else
> firmwares = icssg_emac_firmwares;
>
> - slice = prueth_emac_slice(emac);
> - if (slice < 0) {
> - netdev_err(emac->ndev, "invalid port\n");
> - return -EINVAL;
> - }
> -
> - ret = icssg_config(prueth, emac, slice);
> - if (ret)
> - return ret;
> -
> ret = rproc_set_firmware(prueth->pru[slice], firmwares[slice].pru);
> ret = rproc_boot(prueth->pru[slice]);
> if (ret) {
> @@ -208,7 +198,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> goto halt_rtu;
> }
>
> - emac->fw_running = 1;
> return 0;
>
> halt_rtu:
> @@ -220,6 +209,80 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> return ret;
> }
>
> +static int prueth_emac_common_start(struct prueth *prueth)
> +{
> + struct prueth_emac *emac;
> + int ret = 0;
> + int slice;
> +
> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
> + return -EINVAL;
> +
> + /* clear SMEM and MSMC settings for all slices */
> + memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
> + memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
> +
> + icssg_class_default(prueth->miig_rt, ICSS_SLICE0, 0, false);
> + icssg_class_default(prueth->miig_rt, ICSS_SLICE1, 0, false);
> +
> + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
> + icssg_init_fw_offload_mode(prueth);
> + else
> + icssg_init_emac_mode(prueth);
> +
> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
> + emac = prueth->emac[slice];
> + if (emac) {
> + ret |= icssg_config(prueth, emac, slice);
> + if (ret)
> + return ret;
> + }
> + ret |= prueth_emac_start(prueth, slice);
> + }
> + if (!ret)
> + prueth->prus_running = 1;
> + else
> + return ret;
[Kalesh] This will read better if you change the condition check like:
if (ret)
return ret;
prueth->prus_running = 1;
> +
> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
> + prueth->emac[ICSS_SLICE1];
> + ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
> + emac, IEP_DEFAULT_CYCLE_TIME_NS);
> + if (ret) {
> + dev_err(prueth->dev, "Failed to initialize IEP module\n");
> + return ret;
> + }
> +
> + return 0;
[Kalesh] You can "return ret" here and remove the return from above if
condition.
> +}
> +
> +static int prueth_emac_common_stop(struct prueth *prueth)
> +{
> + struct prueth_emac *emac;
> + int slice;
> +
> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
> + return -EINVAL;
> +
> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE0);
> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE1);
> +
> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
> + if (prueth->prus_running) {
> + rproc_shutdown(prueth->txpru[slice]);
> + rproc_shutdown(prueth->rtu[slice]);
> + rproc_shutdown(prueth->pru[slice]);
> + }
> + }
> + prueth->prus_running = 0;
> +
> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
> + prueth->emac[ICSS_SLICE1];
> + icss_iep_exit(emac->iep);
> +
> + return 0;
> +}
> +
> /* called back by PHY layer if there is change in link state of hw port*/
> static void emac_adjust_link(struct net_device *ndev)
> {
> @@ -369,12 +432,13 @@ static void prueth_iep_settime(void *clockops_data, u64 ns)
> {
> struct icssg_setclock_desc __iomem *sc_descp;
> struct prueth_emac *emac = clockops_data;
> + struct prueth *prueth = emac->prueth;
> struct icssg_setclock_desc sc_desc;
> u64 cyclecount;
> u32 cycletime;
> int timeout;
>
> - if (!emac->fw_running)
> + if (!prueth->prus_running)
> return;
>
> sc_descp = emac->prueth->shram.va + TIMESYNC_FW_WC_SETCLOCK_DESC_OFFSET;
> @@ -543,23 +607,17 @@ static int emac_ndo_open(struct net_device *ndev)
> {
> struct prueth_emac *emac = netdev_priv(ndev);
> int ret, i, num_data_chn = emac->tx_ch_num;
> + struct icssg_flow_cfg __iomem *flow_cfg;
> struct prueth *prueth = emac->prueth;
> int slice = prueth_emac_slice(emac);
> struct device *dev = prueth->dev;
> int max_rx_flows;
> int rx_flow;
>
> - /* clear SMEM and MSMC settings for all slices */
> - if (!prueth->emacs_initialized) {
> - memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
> - memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
> - }
> -
> /* set h/w MAC as user might have re-configured */
> ether_addr_copy(emac->mac_addr, ndev->dev_addr);
>
> icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
> - icssg_class_default(prueth->miig_rt, slice, 0, false);
> icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>
> /* Notify the stack of the actual queue counts. */
> @@ -597,18 +655,23 @@ static int emac_ndo_open(struct net_device *ndev)
> goto cleanup_napi;
> }
>
> - /* reset and start PRU firmware */
> - ret = prueth_emac_start(prueth, emac);
> - if (ret)
> - goto free_rx_irq;
> + if (!prueth->emacs_initialized) {
> + ret = prueth_emac_common_start(prueth);
> + if (ret)
> + goto stop;
> + }
>
> - icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
> + flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
> + writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);
> + ret = emac_fdb_flow_id_updated(emac);
>
> - if (!prueth->emacs_initialized) {
> - ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
> - emac, IEP_DEFAULT_CYCLE_TIME_NS);
> + if (ret) {
> + netdev_err(ndev, "Failed to update Rx Flow ID %d", ret);
> + goto stop;
> }
>
> + icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
> +
> ret = request_threaded_irq(emac->tx_ts_irq, NULL, prueth_tx_ts_irq,
> IRQF_ONESHOT, dev_name(dev), emac);
> if (ret)
> @@ -653,8 +716,7 @@ static int emac_ndo_open(struct net_device *ndev)
> free_tx_ts_irq:
> free_irq(emac->tx_ts_irq, emac);
> stop:
> - prueth_emac_stop(emac);
> -free_rx_irq:
> + prueth_emac_common_stop(prueth);
> free_irq(emac->rx_chns.irq[rx_flow], emac);
> cleanup_napi:
> prueth_ndev_del_tx_napi(emac, emac->tx_ch_num);
> @@ -689,8 +751,6 @@ static int emac_ndo_stop(struct net_device *ndev)
> if (ndev->phydev)
> phy_stop(ndev->phydev);
>
> - icssg_class_disable(prueth->miig_rt, prueth_emac_slice(emac));
> -
> if (emac->prueth->is_hsr_offload_mode)
> __dev_mc_unsync(ndev, icssg_prueth_hsr_del_mcast);
> else
> @@ -728,11 +788,9 @@ static int emac_ndo_stop(struct net_device *ndev)
> /* Destroying the queued work in ndo_stop() */
> cancel_delayed_work_sync(&emac->stats_work);
>
> - if (prueth->emacs_initialized == 1)
> - icss_iep_exit(emac->iep);
> -
> /* stop PRUs */
> - prueth_emac_stop(emac);
> + if (prueth->emacs_initialized == 1)
> + prueth_emac_common_stop(prueth);
>
> free_irq(emac->tx_ts_irq, emac);
>
> @@ -1069,16 +1127,10 @@ static void prueth_emac_restart(struct prueth *prueth)
> icssg_set_port_state(emac1, ICSSG_EMAC_PORT_DISABLE);
>
> /* Stop both pru cores for both PRUeth ports*/
> - prueth_emac_stop(emac0);
> - prueth->emacs_initialized--;
> - prueth_emac_stop(emac1);
> - prueth->emacs_initialized--;
> + prueth_emac_common_stop(prueth);
>
> /* Start both pru cores for both PRUeth ports */
> - prueth_emac_start(prueth, emac0);
> - prueth->emacs_initialized++;
> - prueth_emac_start(prueth, emac1);
> - prueth->emacs_initialized++;
> + prueth_emac_common_start(prueth);
>
> /* Enable forwarding for both PRUeth ports */
> icssg_set_port_state(emac0, ICSSG_EMAC_PORT_FORWARD);
> @@ -1413,13 +1465,10 @@ static int prueth_probe(struct platform_device *pdev)
> prueth->pa_stats = NULL;
> }
>
> - if (eth0_node) {
> + if (eth0_node || eth1_node) {
> ret = prueth_get_cores(prueth, ICSS_SLICE0, false);
> if (ret)
> goto put_cores;
> - }
> -
> - if (eth1_node) {
> ret = prueth_get_cores(prueth, ICSS_SLICE1, false);
> if (ret)
> goto put_cores;
> @@ -1618,14 +1667,12 @@ static int prueth_probe(struct platform_device *pdev)
> pruss_put(prueth->pruss);
>
> put_cores:
> - if (eth1_node) {
> - prueth_put_cores(prueth, ICSS_SLICE1);
> - of_node_put(eth1_node);
> - }
> -
> - if (eth0_node) {
> + if (eth0_node || eth1_node) {
> prueth_put_cores(prueth, ICSS_SLICE0);
> of_node_put(eth0_node);
> +
> + prueth_put_cores(prueth, ICSS_SLICE1);
> + of_node_put(eth1_node);
> }
>
> return ret;
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> index f5c1d473e9f9..b30f2e9a73d8 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> @@ -257,6 +257,7 @@ struct icssg_firmwares {
> * @is_switchmode_supported: indicates platform support for switch mode
> * @switch_id: ID for mapping switch ports to bridge
> * @default_vlan: Default VLAN for host
> + * @prus_running: flag to indicate if all pru cores are running
> */
> struct prueth {
> struct device *dev;
> @@ -298,6 +299,7 @@ struct prueth {
> int default_vlan;
> /** @vtbl_lock: Lock for vtbl in shared memory */
> spinlock_t vtbl_lock;
> + bool prus_running;
> };
>
> struct emac_tx_ts_response {
> @@ -361,6 +363,8 @@ int icssg_set_port_state(struct prueth_emac *emac,
> enum icssg_port_state_cmd state);
> void icssg_config_set_speed(struct prueth_emac *emac);
> void icssg_config_half_duplex(struct prueth_emac *emac);
> +void icssg_init_emac_mode(struct prueth *prueth);
> +void icssg_init_fw_offload_mode(struct prueth *prueth);
>
> /* Buffer queue helpers */
> int icssg_queue_pop(struct prueth *prueth, u8 queue);
> @@ -377,6 +381,7 @@ void icssg_vtbl_modify(struct prueth_emac *emac, u8 vid, u8 port_mask,
> u8 untag_mask, bool add);
> u16 icssg_get_pvid(struct prueth_emac *emac);
> void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port);
> +int emac_fdb_flow_id_updated(struct prueth_emac *emac);
> #define prueth_napi_to_tx_chn(pnapi) \
> container_of(pnapi, struct prueth_tx_chn, napi_tx)
>
> --
> 2.25.1
>
>
--
Regards,
Kalesh A P
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence.
2024-12-05 8:28 ` [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence Meghana Malladi
2024-12-05 9:10 ` Kalesh Anakkur Purayil
@ 2024-12-05 13:08 ` Roger Quadros
2024-12-09 10:34 ` Meghana Malladi
1 sibling, 1 reply; 12+ messages in thread
From: Roger Quadros @ 2024-12-05 13:08 UTC (permalink / raw)
To: Meghana Malladi, vigneshr, jan.kiszka, javier.carrasco.cruz,
diogo.ivo, jacob.e.keller, horms, pabeni, kuba, edumazet, davem,
andrew+netdev
Cc: linux-kernel, netdev, linux-arm-kernel, srk, danishanwar
Hi,
On 05/12/2024 10:28, Meghana Malladi wrote:
> From: MD Danish Anwar <danishanwar@ti.com>
>
> Timesync related operations are ran in PRU0 cores for both ICSSG SLICE0
> and SLICE1. Currently whenever any ICSSG interface comes up we load the
> respective firmwares to PRU cores and whenever interface goes down, we
> stop the resective cores. Due to this, when SLICE0 goes down while
> SLICE1 is still active, PRU0 firmwares are unloaded and PRU0 core is
> stopped. This results in clock jump for SLICE1 interface as the timesync
> related operations are no longer running.
>
> As there are interdependencies between SLICE0 and SLICE1 firmwares,
> fix this by running both PRU0 and PRU1 firmwares as long as at least 1
> ICSSG interface is up. Add new flag in prueth struct to check if all
> firmwares are running.
>
> Use emacs_initialized as reference count to load the firmwares for the
> first and last interface up/down. Moving init_emac_mode and fw_offload_mode
> API outside of icssg_config to icssg_common_start API as they need
> to be called only once per firmware boot.
>
> Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
> ---
>
> Hi all,
>
> This patch is based on net-next tagged next-20241128.
> v2:https://lore.kernel.org/all/20241128122931.2494446-2-m-malladi@ti.com/
>
> * Changes since v2 (v3-v2):
> - error handling in caller function of prueth_emac_common_start()
> - Use prus_running flag check before stopping the firmwares
> Both suggested by Roger Quadros <rogerq@kernel.org>
>
> drivers/net/ethernet/ti/icssg/icssg_config.c | 45 ++++--
> drivers/net/ethernet/ti/icssg/icssg_config.h | 1 +
> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 157 ++++++++++++-------
> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 5 +
> 4 files changed, 140 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
> index 5d2491c2943a..342150756cf7 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
> @@ -397,7 +397,7 @@ static int prueth_emac_buffer_setup(struct prueth_emac *emac)
> return 0;
> }
>
> -static void icssg_init_emac_mode(struct prueth *prueth)
> +void icssg_init_emac_mode(struct prueth *prueth)
> {
> /* When the device is configured as a bridge and it is being brought
> * back to the emac mode, the host mac address has to be set as 0.
> @@ -406,9 +406,6 @@ static void icssg_init_emac_mode(struct prueth *prueth)
> int i;
> u8 mac[ETH_ALEN] = { 0 };
>
> - 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);
> @@ -423,15 +420,13 @@ static void icssg_init_emac_mode(struct prueth *prueth)
> /* Clear host MAC address */
> icssg_class_set_host_mac_addr(prueth->miig_rt, mac);
> }
> +EXPORT_SYMBOL_GPL(icssg_init_emac_mode);
>
> -static void icssg_init_fw_offload_mode(struct prueth *prueth)
> +void icssg_init_fw_offload_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);
> @@ -448,6 +443,7 @@ static void icssg_init_fw_offload_mode(struct prueth *prueth)
> 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);
> }
> +EXPORT_SYMBOL_GPL(icssg_init_fw_offload_mode);
>
> int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
> {
> @@ -455,11 +451,6 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
> struct icssg_flow_cfg __iomem *flow_cfg;
> int ret;
>
> - if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
> - icssg_init_fw_offload_mode(prueth);
> - else
> - icssg_init_emac_mode(prueth);
> -
> memset_io(config, 0, TAS_GATE_MASK_LIST0);
> icssg_miig_queues_init(prueth, slice);
>
> @@ -786,3 +777,31 @@ void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port)
> writel(pvid, prueth->shram.va + EMAC_ICSSG_SWITCH_PORT0_DEFAULT_VLAN_OFFSET);
> }
> EXPORT_SYMBOL_GPL(icssg_set_pvid);
> +
> +int emac_fdb_flow_id_updated(struct prueth_emac *emac)
> +{
> + struct mgmt_cmd_rsp fdb_cmd_rsp = { 0 };
> + int slice = prueth_emac_slice(emac);
> + struct mgmt_cmd fdb_cmd = { 0 };
> + int ret = 0;
> +
> + fdb_cmd.header = ICSSG_FW_MGMT_CMD_HEADER;
> + fdb_cmd.type = ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW;
> + fdb_cmd.seqnum = ++(emac->prueth->icssg_hwcmdseq);
> + fdb_cmd.param = 0;
> +
> + fdb_cmd.param |= (slice << 4);
> + fdb_cmd.cmd_args[0] = 0;
> +
> + ret = icssg_send_fdb_msg(emac, &fdb_cmd, &fdb_cmd_rsp);
> +
> + if (ret)
> + return ret;
> +
> + WARN_ON(fdb_cmd.seqnum != fdb_cmd_rsp.seqnum);
> + if (fdb_cmd_rsp.status == 1)
> + return 0;
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(emac_fdb_flow_id_updated);
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
> index 92c2deaa3068..c884e9fa099e 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
> @@ -55,6 +55,7 @@ struct icssg_rxq_ctx {
> #define ICSSG_FW_MGMT_FDB_CMD_TYPE 0x03
> #define ICSSG_FW_MGMT_CMD_TYPE 0x04
> #define ICSSG_FW_MGMT_PKT 0x80000000
> +#define ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW 0x05
>
> struct icssg_r30_cmd {
> u32 cmd[4];
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index c568c84a032b..2e22e793b01a 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -164,11 +164,11 @@ static struct icssg_firmwares icssg_emac_firmwares[] = {
> }
> };
>
> -static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> +static int prueth_emac_start(struct prueth *prueth, int slice)
> {
> struct icssg_firmwares *firmwares;
> struct device *dev = prueth->dev;
> - int slice, ret;
> + int ret;
>
> if (prueth->is_switch_mode)
> firmwares = icssg_switch_firmwares;
> @@ -177,16 +177,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> else
> firmwares = icssg_emac_firmwares;
>
> - slice = prueth_emac_slice(emac);
> - if (slice < 0) {
> - netdev_err(emac->ndev, "invalid port\n");
> - return -EINVAL;
> - }
> -
> - ret = icssg_config(prueth, emac, slice);
> - if (ret)
> - return ret;
> -
> ret = rproc_set_firmware(prueth->pru[slice], firmwares[slice].pru);
> ret = rproc_boot(prueth->pru[slice]);
> if (ret) {
> @@ -208,7 +198,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> goto halt_rtu;
> }
>
> - emac->fw_running = 1;
> return 0;
>
> halt_rtu:
> @@ -220,6 +209,80 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> return ret;
> }
>
> +static int prueth_emac_common_start(struct prueth *prueth)
> +{
> + struct prueth_emac *emac;
> + int ret = 0;
> + int slice;
> +
> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
> + return -EINVAL;
> +
> + /* clear SMEM and MSMC settings for all slices */
> + memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
> + memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
> +
> + icssg_class_default(prueth->miig_rt, ICSS_SLICE0, 0, false);
> + icssg_class_default(prueth->miig_rt, ICSS_SLICE1, 0, false);
> +
> + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
> + icssg_init_fw_offload_mode(prueth);
> + else
> + icssg_init_emac_mode(prueth);
> +
> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
> + emac = prueth->emac[slice];
> + if (emac) {
> + ret |= icssg_config(prueth, emac, slice);
> + if (ret)
> + return ret;
> + }
> + ret |= prueth_emac_start(prueth, slice);
> + }
need newline?
> + if (!ret)
> + prueth->prus_running = 1;
> + else
> + return ret;
> +
> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
> + prueth->emac[ICSS_SLICE1];
> + ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
> + emac, IEP_DEFAULT_CYCLE_TIME_NS);
> + if (ret) {
> + dev_err(prueth->dev, "Failed to initialize IEP module\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int prueth_emac_common_stop(struct prueth *prueth)
> +{
> + struct prueth_emac *emac;
> + int slice;
> +
> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
> + return -EINVAL;
> +
> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE0);
> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE1);
> +
> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
> + if (prueth->prus_running) {
> + rproc_shutdown(prueth->txpru[slice]);
> + rproc_shutdown(prueth->rtu[slice]);
> + rproc_shutdown(prueth->pru[slice]);
> + }
> + }
> + prueth->prus_running = 0;
> +
> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
> + prueth->emac[ICSS_SLICE1];
> + icss_iep_exit(emac->iep);
if icss_iep_init() failed at prueth_emac_common_start(), we should not be
calling icss_iep_exit(). Maybe you need another flag for iep_init status?
Is it better to call icss_iep_exit() at the top before icssg_class_disable()?
> +
> + return 0;
> +}
> +
> /* called back by PHY layer if there is change in link state of hw port*/
> static void emac_adjust_link(struct net_device *ndev)
> {
> @@ -369,12 +432,13 @@ static void prueth_iep_settime(void *clockops_data, u64 ns)
> {
> struct icssg_setclock_desc __iomem *sc_descp;
> struct prueth_emac *emac = clockops_data;
> + struct prueth *prueth = emac->prueth;
> struct icssg_setclock_desc sc_desc;
> u64 cyclecount;
> u32 cycletime;
> int timeout;
>
> - if (!emac->fw_running)
> + if (!prueth->prus_running)
> return;
>
> sc_descp = emac->prueth->shram.va + TIMESYNC_FW_WC_SETCLOCK_DESC_OFFSET;
> @@ -543,23 +607,17 @@ static int emac_ndo_open(struct net_device *ndev)
> {
> struct prueth_emac *emac = netdev_priv(ndev);
> int ret, i, num_data_chn = emac->tx_ch_num;
> + struct icssg_flow_cfg __iomem *flow_cfg;
> struct prueth *prueth = emac->prueth;
> int slice = prueth_emac_slice(emac);
> struct device *dev = prueth->dev;
> int max_rx_flows;
> int rx_flow;
>
> - /* clear SMEM and MSMC settings for all slices */
> - if (!prueth->emacs_initialized) {
> - memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
> - memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
> - }
> -
> /* set h/w MAC as user might have re-configured */
> ether_addr_copy(emac->mac_addr, ndev->dev_addr);
>
> icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
> - icssg_class_default(prueth->miig_rt, slice, 0, false);
> icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>
> /* Notify the stack of the actual queue counts. */
> @@ -597,18 +655,23 @@ static int emac_ndo_open(struct net_device *ndev)
> goto cleanup_napi;
> }
>
> - /* reset and start PRU firmware */
> - ret = prueth_emac_start(prueth, emac);
> - if (ret)
> - goto free_rx_irq;
> + if (!prueth->emacs_initialized) {
> + ret = prueth_emac_common_start(prueth);
> + if (ret)
> + goto stop;
> + }
>
> - icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
> + flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
> + writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);
> + ret = emac_fdb_flow_id_updated(emac);
>
> - if (!prueth->emacs_initialized) {
> - ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
> - emac, IEP_DEFAULT_CYCLE_TIME_NS);
> + if (ret) {
> + netdev_err(ndev, "Failed to update Rx Flow ID %d", ret);
> + goto stop;
> }
>
> + icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
> +
> ret = request_threaded_irq(emac->tx_ts_irq, NULL, prueth_tx_ts_irq,
> IRQF_ONESHOT, dev_name(dev), emac);
> if (ret)
> @@ -653,8 +716,7 @@ static int emac_ndo_open(struct net_device *ndev)
> free_tx_ts_irq:
> free_irq(emac->tx_ts_irq, emac);
> stop:
> - prueth_emac_stop(emac);
> -free_rx_irq:
> + prueth_emac_common_stop(prueth);
> free_irq(emac->rx_chns.irq[rx_flow], emac);
> cleanup_napi:
> prueth_ndev_del_tx_napi(emac, emac->tx_ch_num);
> @@ -689,8 +751,6 @@ static int emac_ndo_stop(struct net_device *ndev)
> if (ndev->phydev)
> phy_stop(ndev->phydev);
>
> - icssg_class_disable(prueth->miig_rt, prueth_emac_slice(emac));
> -
> if (emac->prueth->is_hsr_offload_mode)
> __dev_mc_unsync(ndev, icssg_prueth_hsr_del_mcast);
> else
> @@ -728,11 +788,9 @@ static int emac_ndo_stop(struct net_device *ndev)
> /* Destroying the queued work in ndo_stop() */
> cancel_delayed_work_sync(&emac->stats_work);
>
> - if (prueth->emacs_initialized == 1)
> - icss_iep_exit(emac->iep);
> -
> /* stop PRUs */
> - prueth_emac_stop(emac);
> + if (prueth->emacs_initialized == 1)
> + prueth_emac_common_stop(prueth);
>
> free_irq(emac->tx_ts_irq, emac);
>
> @@ -1069,16 +1127,10 @@ static void prueth_emac_restart(struct prueth *prueth)
> icssg_set_port_state(emac1, ICSSG_EMAC_PORT_DISABLE);
>
> /* Stop both pru cores for both PRUeth ports*/
> - prueth_emac_stop(emac0);
> - prueth->emacs_initialized--;
> - prueth_emac_stop(emac1);
> - prueth->emacs_initialized--;
> + prueth_emac_common_stop(prueth);
>
> /* Start both pru cores for both PRUeth ports */
> - prueth_emac_start(prueth, emac0);
> - prueth->emacs_initialized++;
> - prueth_emac_start(prueth, emac1);
> - prueth->emacs_initialized++;
> + prueth_emac_common_start(prueth);
But this can fail? You need to deal with failure condition appropriately.
>
> /* Enable forwarding for both PRUeth ports */
> icssg_set_port_state(emac0, ICSSG_EMAC_PORT_FORWARD);
> @@ -1413,13 +1465,10 @@ static int prueth_probe(struct platform_device *pdev)
> prueth->pa_stats = NULL;
> }
>
> - if (eth0_node) {
> + if (eth0_node || eth1_node) {
> ret = prueth_get_cores(prueth, ICSS_SLICE0, false);
> if (ret)
> goto put_cores;
> - }
> -
> - if (eth1_node) {
> ret = prueth_get_cores(prueth, ICSS_SLICE1, false);
> if (ret)
> goto put_cores;
> @@ -1618,14 +1667,12 @@ static int prueth_probe(struct platform_device *pdev)
> pruss_put(prueth->pruss);
>
> put_cores:
> - if (eth1_node) {
> - prueth_put_cores(prueth, ICSS_SLICE1);
> - of_node_put(eth1_node);
> - }
> -
> - if (eth0_node) {
> + if (eth0_node || eth1_node) {
> prueth_put_cores(prueth, ICSS_SLICE0);
> of_node_put(eth0_node);
> +
> + prueth_put_cores(prueth, ICSS_SLICE1);
> + of_node_put(eth1_node);
> }
>
> return ret;
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> index f5c1d473e9f9..b30f2e9a73d8 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> @@ -257,6 +257,7 @@ struct icssg_firmwares {
> * @is_switchmode_supported: indicates platform support for switch mode
> * @switch_id: ID for mapping switch ports to bridge
> * @default_vlan: Default VLAN for host
> + * @prus_running: flag to indicate if all pru cores are running
> */
> struct prueth {
> struct device *dev;
> @@ -298,6 +299,7 @@ struct prueth {
> int default_vlan;
> /** @vtbl_lock: Lock for vtbl in shared memory */
> spinlock_t vtbl_lock;
> + bool prus_running;
I think you don't need fw_running flag anymore. Could you please remove it
from struct prueth_emac?
> };
>
> struct emac_tx_ts_response {
> @@ -361,6 +363,8 @@ int icssg_set_port_state(struct prueth_emac *emac,
> enum icssg_port_state_cmd state);
> void icssg_config_set_speed(struct prueth_emac *emac);
> void icssg_config_half_duplex(struct prueth_emac *emac);
> +void icssg_init_emac_mode(struct prueth *prueth);
> +void icssg_init_fw_offload_mode(struct prueth *prueth);
>
> /* Buffer queue helpers */
> int icssg_queue_pop(struct prueth *prueth, u8 queue);
> @@ -377,6 +381,7 @@ void icssg_vtbl_modify(struct prueth_emac *emac, u8 vid, u8 port_mask,
> u8 untag_mask, bool add);
> u16 icssg_get_pvid(struct prueth_emac *emac);
> void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port);
> +int emac_fdb_flow_id_updated(struct prueth_emac *emac);
> #define prueth_napi_to_tx_chn(pnapi) \
> container_of(pnapi, struct prueth_tx_chn, napi_tx)
>
--
cheers,
-roger
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence.
2024-12-05 13:08 ` Roger Quadros
@ 2024-12-09 10:34 ` Meghana Malladi
2024-12-09 12:39 ` Roger Quadros
2024-12-09 14:44 ` Diogo Ivo
0 siblings, 2 replies; 12+ messages in thread
From: Meghana Malladi @ 2024-12-09 10:34 UTC (permalink / raw)
To: Roger Quadros, vigneshr, jan.kiszka, javier.carrasco.cruz,
diogo.ivo, jacob.e.keller, horms, pabeni, kuba, edumazet, davem,
andrew+netdev
Cc: linux-kernel, netdev, linux-arm-kernel, srk, danishanwar
On 05/12/24 18:38, Roger Quadros wrote:
> Hi,
>
> On 05/12/2024 10:28, Meghana Malladi wrote:
>> From: MD Danish Anwar <danishanwar@ti.com>
>>
>> Timesync related operations are ran in PRU0 cores for both ICSSG SLICE0
>> and SLICE1. Currently whenever any ICSSG interface comes up we load the
>> respective firmwares to PRU cores and whenever interface goes down, we
>> stop the resective cores. Due to this, when SLICE0 goes down while
>> SLICE1 is still active, PRU0 firmwares are unloaded and PRU0 core is
>> stopped. This results in clock jump for SLICE1 interface as the timesync
>> related operations are no longer running.
>>
>> As there are interdependencies between SLICE0 and SLICE1 firmwares,
>> fix this by running both PRU0 and PRU1 firmwares as long as at least 1
>> ICSSG interface is up. Add new flag in prueth struct to check if all
>> firmwares are running.
>>
>> Use emacs_initialized as reference count to load the firmwares for the
>> first and last interface up/down. Moving init_emac_mode and fw_offload_mode
>> API outside of icssg_config to icssg_common_start API as they need
>> to be called only once per firmware boot.
>>
>> Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
>> ---
>>
>> Hi all,
>>
>> This patch is based on net-next tagged next-20241128.
>> v2:https://lore.kernel.org/all/20241128122931.2494446-2-m-malladi@ti.com/
>>
>> * Changes since v2 (v3-v2):
>> - error handling in caller function of prueth_emac_common_start()
>> - Use prus_running flag check before stopping the firmwares
>> Both suggested by Roger Quadros <rogerq@kernel.org>
>>
>> drivers/net/ethernet/ti/icssg/icssg_config.c | 45 ++++--
>> drivers/net/ethernet/ti/icssg/icssg_config.h | 1 +
>> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 157 ++++++++++++-------
>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 5 +
>> 4 files changed, 140 insertions(+), 68 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
>> index 5d2491c2943a..342150756cf7 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
>> @@ -397,7 +397,7 @@ static int prueth_emac_buffer_setup(struct prueth_emac *emac)
>> return 0;
>> }
>>
[ ... ]
>> +static int prueth_emac_common_start(struct prueth *prueth)
>> +{
>> + struct prueth_emac *emac;
>> + int ret = 0;
>> + int slice;
>> +
>> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
>> + return -EINVAL;
>> +
>> + /* clear SMEM and MSMC settings for all slices */
>> + memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
>> + memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
>> +
>> + icssg_class_default(prueth->miig_rt, ICSS_SLICE0, 0, false);
>> + icssg_class_default(prueth->miig_rt, ICSS_SLICE1, 0, false);
>> +
>> + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
>> + icssg_init_fw_offload_mode(prueth);
>> + else
>> + icssg_init_emac_mode(prueth);
>> +
>> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
>> + emac = prueth->emac[slice];
>> + if (emac) {
>> + ret |= icssg_config(prueth, emac, slice);
>> + if (ret)
>> + return ret;
>> + }
>> + ret |= prueth_emac_start(prueth, slice);
>> + }
>
> need newline?
>
Yes I will add it.
>> + if (!ret)
>> + prueth->prus_running = 1;
>> + else
>> + return ret;
>> +
>> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
>> + prueth->emac[ICSS_SLICE1];
>> + ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
>> + emac, IEP_DEFAULT_CYCLE_TIME_NS);
>> + if (ret) {
>> + dev_err(prueth->dev, "Failed to initialize IEP module\n");
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int prueth_emac_common_stop(struct prueth *prueth)
>> +{
>> + struct prueth_emac *emac;
>> + int slice;
>> +
>> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
>> + return -EINVAL;
>> +
>> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE0);
>> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE1);
>> +
>> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
>> + if (prueth->prus_running) {
>> + rproc_shutdown(prueth->txpru[slice]);
>> + rproc_shutdown(prueth->rtu[slice]);
>> + rproc_shutdown(prueth->pru[slice]);
>> + }
>> + }
>> + prueth->prus_running = 0;
>> +
>> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
>> + prueth->emac[ICSS_SLICE1];
>> + icss_iep_exit(emac->iep);
>
> if icss_iep_init() failed at prueth_emac_common_start(), we should not be
> calling icss_iep_exit(). Maybe you need another flag for iep_init status?
>
Yes I have thought of it as well. In icss_iep_init() does lot of iep
register configuration and in the end it enables iep by setting
IEP_CNT_ENABLE bit and ptp_clock_register(). Whereas in icss_iep_exit()
it checks for ptp_clock and pps/perout. And calls icss_iep_disable()
which again clears IEP_CNT_ENABLE.
So I see no harm in calling icss_iep_exit() even if icss_iep_init() as
it overwrites the existing configuration only. But if you think this
doesn't look good I can definitely add new flag for iep as well. But IMO
I think this flag would be redundant, please correct me if I am wrong.
So which one sounds better?
> Is it better to call icss_iep_exit() at the top before icssg_class_disable()?
>
>> +
>> + return 0;
>> +}
>> +
>> /* called back by PHY layer if there is change in link state of hw port*/
>> static void emac_adjust_link(struct net_device *ndev)
>> {
>> @@ -369,12 +432,13 @@ static void prueth_iep_settime(void *clockops_data, u64 ns)
>> {
>> struct icssg_setclock_desc __iomem *sc_descp;
>> struct prueth_emac *emac = clockops_data;
>> + struct prueth *prueth = emac->prueth;
>> struct icssg_setclock_desc sc_desc;
>> u64 cyclecount;
>> u32 cycletime;
>> int timeout;
>>
>> - if (!emac->fw_running)
>> + if (!prueth->prus_running)
>> return;
>>
>> sc_descp = emac->prueth->shram.va + TIMESYNC_FW_WC_SETCLOCK_DESC_OFFSET;
>> @@ -543,23 +607,17 @@ static int emac_ndo_open(struct net_device *ndev)
>> {
>> struct prueth_emac *emac = netdev_priv(ndev);
>> int ret, i, num_data_chn = emac->tx_ch_num;
>> + struct icssg_flow_cfg __iomem *flow_cfg;
>> struct prueth *prueth = emac->prueth;
>> int slice = prueth_emac_slice(emac);
>> struct device *dev = prueth->dev;
>> int max_rx_flows;
>> int rx_flow;
>>
>> - /* clear SMEM and MSMC settings for all slices */
>> - if (!prueth->emacs_initialized) {
>> - memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
>> - memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
>> - }
>> -
>> /* set h/w MAC as user might have re-configured */
>> ether_addr_copy(emac->mac_addr, ndev->dev_addr);
>>
>> icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>> - icssg_class_default(prueth->miig_rt, slice, 0, false);
>> icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>>
>> /* Notify the stack of the actual queue counts. */
>> @@ -597,18 +655,23 @@ static int emac_ndo_open(struct net_device *ndev)
>> goto cleanup_napi;
>> }
>>
>> - /* reset and start PRU firmware */
>> - ret = prueth_emac_start(prueth, emac);
>> - if (ret)
>> - goto free_rx_irq;
>> + if (!prueth->emacs_initialized) {
>> + ret = prueth_emac_common_start(prueth);
>> + if (ret)
>> + goto stop;
>> + }
>>
>> - icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
>> + flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
>> + writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);
>> + ret = emac_fdb_flow_id_updated(emac);
>>
>> - if (!prueth->emacs_initialized) {
>> - ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
>> - emac, IEP_DEFAULT_CYCLE_TIME_NS);
>> + if (ret) {
>> + netdev_err(ndev, "Failed to update Rx Flow ID %d", ret);
>> + goto stop;
>> }
>>
>> + icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
>> +
>> ret = request_threaded_irq(emac->tx_ts_irq, NULL, prueth_tx_ts_irq,
>> IRQF_ONESHOT, dev_name(dev), emac);
>> if (ret)
>> @@ -653,8 +716,7 @@ static int emac_ndo_open(struct net_device *ndev)
>> free_tx_ts_irq:
>> free_irq(emac->tx_ts_irq, emac);
>> stop:
>> - prueth_emac_stop(emac);
>> -free_rx_irq:
>> + prueth_emac_common_stop(prueth);
>> free_irq(emac->rx_chns.irq[rx_flow], emac);
>> cleanup_napi:
>> prueth_ndev_del_tx_napi(emac, emac->tx_ch_num);
>> @@ -689,8 +751,6 @@ static int emac_ndo_stop(struct net_device *ndev)
>> if (ndev->phydev)
>> phy_stop(ndev->phydev);
>>
>> - icssg_class_disable(prueth->miig_rt, prueth_emac_slice(emac));
>> -
>> if (emac->prueth->is_hsr_offload_mode)
>> __dev_mc_unsync(ndev, icssg_prueth_hsr_del_mcast);
>> else
>> @@ -728,11 +788,9 @@ static int emac_ndo_stop(struct net_device *ndev)
>> /* Destroying the queued work in ndo_stop() */
>> cancel_delayed_work_sync(&emac->stats_work);
>>
>> - if (prueth->emacs_initialized == 1)
>> - icss_iep_exit(emac->iep);
>> -
>> /* stop PRUs */
>> - prueth_emac_stop(emac);
>> + if (prueth->emacs_initialized == 1)
>> + prueth_emac_common_stop(prueth);
>>
>> free_irq(emac->tx_ts_irq, emac);
>>
>> @@ -1069,16 +1127,10 @@ static void prueth_emac_restart(struct prueth *prueth)
>> icssg_set_port_state(emac1, ICSSG_EMAC_PORT_DISABLE);
>>
>> /* Stop both pru cores for both PRUeth ports*/
>> - prueth_emac_stop(emac0);
>> - prueth->emacs_initialized--;
>> - prueth_emac_stop(emac1);
>> - prueth->emacs_initialized--;
>> + prueth_emac_common_stop(prueth);
>>
>> /* Start both pru cores for both PRUeth ports */
>> - prueth_emac_start(prueth, emac0);
>> - prueth->emacs_initialized++;
>> - prueth_emac_start(prueth, emac1);
>> - prueth->emacs_initialized++;
>> + prueth_emac_common_start(prueth);
>
> But this can fail? You need to deal with failure condition appropriately.
>
I haven't added failure conditions for two reasons:
- Existing code also didn't have any error checks
- This func simply reloads a new firmware, given everything is already
working with the old one.
I can still handle error cases by changing this func to return int
(currently it is void) and caller of the functions should print error
and immediately return. Thoughts on this?
>>
>> /* Enable forwarding for both PRUeth ports */
>> icssg_set_port_state(emac0, ICSSG_EMAC_PORT_FORWARD);
>> @@ -1413,13 +1465,10 @@ static int prueth_probe(struct platform_device *pdev)
>> prueth->pa_stats = NULL;
>> }
>>
>> - if (eth0_node) {
>> + if (eth0_node || eth1_node) {
>> ret = prueth_get_cores(prueth, ICSS_SLICE0, false);
>> if (ret)
>> goto put_cores;
>> - }
>> -
>> - if (eth1_node) {
>> ret = prueth_get_cores(prueth, ICSS_SLICE1, false);
>> if (ret)
>> goto put_cores;
>> @@ -1618,14 +1667,12 @@ static int prueth_probe(struct platform_device *pdev)
>> pruss_put(prueth->pruss);
>>
>> put_cores:
>> - if (eth1_node) {
>> - prueth_put_cores(prueth, ICSS_SLICE1);
>> - of_node_put(eth1_node);
>> - }
>> -
>> - if (eth0_node) {
>> + if (eth0_node || eth1_node) {
>> prueth_put_cores(prueth, ICSS_SLICE0);
>> of_node_put(eth0_node);
>> +
>> + prueth_put_cores(prueth, ICSS_SLICE1);
>> + of_node_put(eth1_node);
>> }
>>
>> return ret;
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> index f5c1d473e9f9..b30f2e9a73d8 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> @@ -257,6 +257,7 @@ struct icssg_firmwares {
>> * @is_switchmode_supported: indicates platform support for switch mode
>> * @switch_id: ID for mapping switch ports to bridge
>> * @default_vlan: Default VLAN for host
>> + * @prus_running: flag to indicate if all pru cores are running
>> */
>> struct prueth {
>> struct device *dev;
>> @@ -298,6 +299,7 @@ struct prueth {
>> int default_vlan;
>> /** @vtbl_lock: Lock for vtbl in shared memory */
>> spinlock_t vtbl_lock;
>> + bool prus_running;
>
> I think you don't need fw_running flag anymore. Could you please remove it
> from struct prueth_emac?
>
This flag is still being used by SR1, for which this patch doesn't
apply. So I prefer not touching this flag for the sake of SR1.
>> };
>>
>> struct emac_tx_ts_response {
>> @@ -361,6 +363,8 @@ int icssg_set_port_state(struct prueth_emac *emac,
>> enum icssg_port_state_cmd state);
>> void icssg_config_set_speed(struct prueth_emac *emac);
>> void icssg_config_half_duplex(struct prueth_emac *emac);
>> +void icssg_init_emac_mode(struct prueth *prueth);
>> +void icssg_init_fw_offload_mode(struct prueth *prueth);
>>
>> /* Buffer queue helpers */
>> int icssg_queue_pop(struct prueth *prueth, u8 queue);
>> @@ -377,6 +381,7 @@ void icssg_vtbl_modify(struct prueth_emac *emac, u8 vid, u8 port_mask,
>> u8 untag_mask, bool add);
>> u16 icssg_get_pvid(struct prueth_emac *emac);
>> void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port);
>> +int emac_fdb_flow_id_updated(struct prueth_emac *emac);
>> #define prueth_napi_to_tx_chn(pnapi) \
>> container_of(pnapi, struct prueth_tx_chn, napi_tx)
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [EXTERNAL] Re: [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence.
2024-12-05 9:10 ` Kalesh Anakkur Purayil
@ 2024-12-09 10:39 ` Meghana Malladi
0 siblings, 0 replies; 12+ messages in thread
From: Meghana Malladi @ 2024-12-09 10:39 UTC (permalink / raw)
To: Kalesh Anakkur Purayil
Cc: vigneshr, jan.kiszka, Roger Quadros, javier.carrasco.cruz,
diogo.ivo, jacob.e.keller, horms, pabeni, kuba, edumazet, davem,
andrew+netdev, linux-kernel, netdev, linux-arm-kernel, srk,
danishanwar
On 05/12/24 14:40, Kalesh Anakkur Purayil wrote:
> On Thu, Dec 5, 2024 at 1:59 PM Meghana Malladi <m-malladi@ti.com> wrote:
>>
>> From: MD Danish Anwar <danishanwar@ti.com>
>>
>> Timesync related operations are ran in PRU0 cores for both ICSSG SLICE0
>> and SLICE1. Currently whenever any ICSSG interface comes up we load the
>> respective firmwares to PRU cores and whenever interface goes down, we
>> stop the resective cores. Due to this, when SLICE0 goes down while
>> SLICE1 is still active, PRU0 firmwares are unloaded and PRU0 core is
>> stopped. This results in clock jump for SLICE1 interface as the timesync
>> related operations are no longer running.
>>
>> As there are interdependencies between SLICE0 and SLICE1 firmwares,
>> fix this by running both PRU0 and PRU1 firmwares as long as at least 1
>> ICSSG interface is up. Add new flag in prueth struct to check if all
>> firmwares are running.
>>
>> Use emacs_initialized as reference count to load the firmwares for the
>> first and last interface up/down. Moving init_emac_mode and fw_offload_mode
>> API outside of icssg_config to icssg_common_start API as they need
>> to be called only once per firmware boot.
>>
>> Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
>> ---
>>
>> Hi all,
>>
>> This patch is based on net-next tagged next-20241128.
>> v2:https://lore.kernel.org/all/20241128122931.2494446-2-m-malladi@ti.com/
>>
>> * Changes since v2 (v3-v2):
>> - error handling in caller function of prueth_emac_common_start()
>> - Use prus_running flag check before stopping the firmwares
>> Both suggested by Roger Quadros <rogerq@kernel.org>
>>
>> drivers/net/ethernet/ti/icssg/icssg_config.c | 45 ++++--
>> drivers/net/ethernet/ti/icssg/icssg_config.h | 1 +
>> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 157 ++++++++++++-------
>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 5 +
>> 4 files changed, 140 insertions(+), 68 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
>> index 5d2491c2943a..342150756cf7 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
>> @@ -397,7 +397,7 @@ static int prueth_emac_buffer_setup(struct prueth_emac *emac)
>> return 0;
>> }
>>
>> -static void icssg_init_emac_mode(struct prueth *prueth)
>> +void icssg_init_emac_mode(struct prueth *prueth)
>> {
>> /* When the device is configured as a bridge and it is being brought
>> * back to the emac mode, the host mac address has to be set as 0.
>> @@ -406,9 +406,6 @@ static void icssg_init_emac_mode(struct prueth *prueth)
>> int i;
>> u8 mac[ETH_ALEN] = { 0 };
>>
>> - 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);
>> @@ -423,15 +420,13 @@ static void icssg_init_emac_mode(struct prueth *prueth)
>> /* Clear host MAC address */
>> icssg_class_set_host_mac_addr(prueth->miig_rt, mac);
>> }
>> +EXPORT_SYMBOL_GPL(icssg_init_emac_mode);
>>
>> -static void icssg_init_fw_offload_mode(struct prueth *prueth)
>> +void icssg_init_fw_offload_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);
>> @@ -448,6 +443,7 @@ static void icssg_init_fw_offload_mode(struct prueth *prueth)
>> 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);
>> }
>> +EXPORT_SYMBOL_GPL(icssg_init_fw_offload_mode);
>>
>> int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
>> {
>> @@ -455,11 +451,6 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
>> struct icssg_flow_cfg __iomem *flow_cfg;
>> int ret;
>>
>> - if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
>> - icssg_init_fw_offload_mode(prueth);
>> - else
>> - icssg_init_emac_mode(prueth);
>> -
>> memset_io(config, 0, TAS_GATE_MASK_LIST0);
>> icssg_miig_queues_init(prueth, slice);
>>
>> @@ -786,3 +777,31 @@ void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port)
>> writel(pvid, prueth->shram.va + EMAC_ICSSG_SWITCH_PORT0_DEFAULT_VLAN_OFFSET);
>> }
>> EXPORT_SYMBOL_GPL(icssg_set_pvid);
>> +
>> +int emac_fdb_flow_id_updated(struct prueth_emac *emac)
>> +{
>> + struct mgmt_cmd_rsp fdb_cmd_rsp = { 0 };
>> + int slice = prueth_emac_slice(emac);
>> + struct mgmt_cmd fdb_cmd = { 0 };
>> + int ret = 0;
> [Kalesh] There is no need to initialize "ret" here
Yes, I will remove it.
>> +
>> + fdb_cmd.header = ICSSG_FW_MGMT_CMD_HEADER;
>> + fdb_cmd.type = ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW;
>> + fdb_cmd.seqnum = ++(emac->prueth->icssg_hwcmdseq);
>> + fdb_cmd.param = 0;
>> +
>> + fdb_cmd.param |= (slice << 4);
>> + fdb_cmd.cmd_args[0] = 0;
>> +
>> + ret = icssg_send_fdb_msg(emac, &fdb_cmd, &fdb_cmd_rsp);
>> +
> [Kalesh] There is no need of an new line here
Ok, I will remove it.
>> + if (ret)
>> + return ret;
>> +
>> + WARN_ON(fdb_cmd.seqnum != fdb_cmd_rsp.seqnum);
>> + if (fdb_cmd_rsp.status == 1)
>> + return 0;
>> +
>> + return -EINVAL;
> [Kalesh] Maybe you can simplify this as:
> return fdb_cmd_rsp.status == 1 ? 0 : -EINVAL;
Yes, this looks good. I will update it.
>> +}
>> +EXPORT_SYMBOL_GPL(emac_fdb_flow_id_updated);
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
>> index 92c2deaa3068..c884e9fa099e 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
>> @@ -55,6 +55,7 @@ struct icssg_rxq_ctx {
>> #define ICSSG_FW_MGMT_FDB_CMD_TYPE 0x03
>> #define ICSSG_FW_MGMT_CMD_TYPE 0x04
>> #define ICSSG_FW_MGMT_PKT 0x80000000
>> +#define ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW 0x05
>>
>> struct icssg_r30_cmd {
>> u32 cmd[4];
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>> index c568c84a032b..2e22e793b01a 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>> @@ -164,11 +164,11 @@ static struct icssg_firmwares icssg_emac_firmwares[] = {
>> }
>> };
>>
>> -static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
>> +static int prueth_emac_start(struct prueth *prueth, int slice)
>> {
>> struct icssg_firmwares *firmwares;
>> struct device *dev = prueth->dev;
>> - int slice, ret;
>> + int ret;
>>
>> if (prueth->is_switch_mode)
>> firmwares = icssg_switch_firmwares;
>> @@ -177,16 +177,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
>> else
>> firmwares = icssg_emac_firmwares;
>>
>> - slice = prueth_emac_slice(emac);
>> - if (slice < 0) {
>> - netdev_err(emac->ndev, "invalid port\n");
>> - return -EINVAL;
>> - }
>> -
>> - ret = icssg_config(prueth, emac, slice);
>> - if (ret)
>> - return ret;
>> -
>> ret = rproc_set_firmware(prueth->pru[slice], firmwares[slice].pru);
>> ret = rproc_boot(prueth->pru[slice]);
>> if (ret) {
>> @@ -208,7 +198,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
>> goto halt_rtu;
>> }
>>
>> - emac->fw_running = 1;
>> return 0;
>>
>> halt_rtu:
>> @@ -220,6 +209,80 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
>> return ret;
>> }
>>
>> +static int prueth_emac_common_start(struct prueth *prueth)
>> +{
>> + struct prueth_emac *emac;
>> + int ret = 0;
>> + int slice;
>> +
>> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
>> + return -EINVAL;
>> +
>> + /* clear SMEM and MSMC settings for all slices */
>> + memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
>> + memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
>> +
>> + icssg_class_default(prueth->miig_rt, ICSS_SLICE0, 0, false);
>> + icssg_class_default(prueth->miig_rt, ICSS_SLICE1, 0, false);
>> +
>> + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
>> + icssg_init_fw_offload_mode(prueth);
>> + else
>> + icssg_init_emac_mode(prueth);
>> +
>> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
>> + emac = prueth->emac[slice];
>> + if (emac) {
>> + ret |= icssg_config(prueth, emac, slice);
>> + if (ret)
>> + return ret;
>> + }
>> + ret |= prueth_emac_start(prueth, slice);
>> + }
>> + if (!ret)
>> + prueth->prus_running = 1;
>> + else
>> + return ret;
> [Kalesh] This will read better if you change the condition check like:
> if (ret)
> return ret;
> prueth->prus_running = 1;
Thanks, I will update it.
>> +
>> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
>> + prueth->emac[ICSS_SLICE1];
>> + ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
>> + emac, IEP_DEFAULT_CYCLE_TIME_NS);
>> + if (ret) {
>> + dev_err(prueth->dev, "Failed to initialize IEP module\n");
>> + return ret;
>> + }
>> +
>> + return 0;
> [Kalesh] You can "return ret" here and remove the return from above if
> condition.
I will update it.
>> +}
>> +
>> +static int prueth_emac_common_stop(struct prueth *prueth)
>> +{
>> + struct prueth_emac *emac;
>> + int slice;
>> +
>> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
>> + return -EINVAL;
>> +
>> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE0);
>> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE1);
>> +
>> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
>> + if (prueth->prus_running) {
>> + rproc_shutdown(prueth->txpru[slice]);
>> + rproc_shutdown(prueth->rtu[slice]);
>> + rproc_shutdown(prueth->pru[slice]);
>> + }
>> + }
>> + prueth->prus_running = 0;
>> +
>> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
>> + prueth->emac[ICSS_SLICE1];
>> + icss_iep_exit(emac->iep);
>> +
>> + return 0;
>> +}
>> +
>> /* called back by PHY layer if there is change in link state of hw port*/
>> static void emac_adjust_link(struct net_device *ndev)
>> {
>> @@ -369,12 +432,13 @@ static void prueth_iep_settime(void *clockops_data, u64 ns)
>> {
>> struct icssg_setclock_desc __iomem *sc_descp;
>> struct prueth_emac *emac = clockops_data;
>> + struct prueth *prueth = emac->prueth;
>> struct icssg_setclock_desc sc_desc;
>> u64 cyclecount;
>> u32 cycletime;
>> int timeout;
>>
>> - if (!emac->fw_running)
>> + if (!prueth->prus_running)
>> return;
>>
>> sc_descp = emac->prueth->shram.va + TIMESYNC_FW_WC_SETCLOCK_DESC_OFFSET;
>> @@ -543,23 +607,17 @@ static int emac_ndo_open(struct net_device *ndev)
>> {
>> struct prueth_emac *emac = netdev_priv(ndev);
>> int ret, i, num_data_chn = emac->tx_ch_num;
>> + struct icssg_flow_cfg __iomem *flow_cfg;
>> struct prueth *prueth = emac->prueth;
>> int slice = prueth_emac_slice(emac);
>> struct device *dev = prueth->dev;
>> int max_rx_flows;
>> int rx_flow;
>>
>> - /* clear SMEM and MSMC settings for all slices */
>> - if (!prueth->emacs_initialized) {
>> - memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
>> - memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
>> - }
>> -
>> /* set h/w MAC as user might have re-configured */
>> ether_addr_copy(emac->mac_addr, ndev->dev_addr);
>>
>> icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>> - icssg_class_default(prueth->miig_rt, slice, 0, false);
>> icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>>
>> /* Notify the stack of the actual queue counts. */
>> @@ -597,18 +655,23 @@ static int emac_ndo_open(struct net_device *ndev)
>> goto cleanup_napi;
>> }
>>
>> - /* reset and start PRU firmware */
>> - ret = prueth_emac_start(prueth, emac);
>> - if (ret)
>> - goto free_rx_irq;
>> + if (!prueth->emacs_initialized) {
>> + ret = prueth_emac_common_start(prueth);
>> + if (ret)
>> + goto stop;
>> + }
>>
>> - icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
>> + flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
>> + writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);
>> + ret = emac_fdb_flow_id_updated(emac);
>>
>> - if (!prueth->emacs_initialized) {
>> - ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
>> - emac, IEP_DEFAULT_CYCLE_TIME_NS);
>> + if (ret) {
>> + netdev_err(ndev, "Failed to update Rx Flow ID %d", ret);
>> + goto stop;
>> }
>>
>> + icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
>> +
>> ret = request_threaded_irq(emac->tx_ts_irq, NULL, prueth_tx_ts_irq,
>> IRQF_ONESHOT, dev_name(dev), emac);
>> if (ret)
>> @@ -653,8 +716,7 @@ static int emac_ndo_open(struct net_device *ndev)
>> free_tx_ts_irq:
>> free_irq(emac->tx_ts_irq, emac);
>> stop:
>> - prueth_emac_stop(emac);
>> -free_rx_irq:
>> + prueth_emac_common_stop(prueth);
>> free_irq(emac->rx_chns.irq[rx_flow], emac);
>> cleanup_napi:
>> prueth_ndev_del_tx_napi(emac, emac->tx_ch_num);
>> @@ -689,8 +751,6 @@ static int emac_ndo_stop(struct net_device *ndev)
>> if (ndev->phydev)
>> phy_stop(ndev->phydev);
>>
>> - icssg_class_disable(prueth->miig_rt, prueth_emac_slice(emac));
>> -
>> if (emac->prueth->is_hsr_offload_mode)
>> __dev_mc_unsync(ndev, icssg_prueth_hsr_del_mcast);
>> else
>> @@ -728,11 +788,9 @@ static int emac_ndo_stop(struct net_device *ndev)
>> /* Destroying the queued work in ndo_stop() */
>> cancel_delayed_work_sync(&emac->stats_work);
>>
>> - if (prueth->emacs_initialized == 1)
>> - icss_iep_exit(emac->iep);
>> -
>> /* stop PRUs */
>> - prueth_emac_stop(emac);
>> + if (prueth->emacs_initialized == 1)
>> + prueth_emac_common_stop(prueth);
>>
>> free_irq(emac->tx_ts_irq, emac);
>>
>> @@ -1069,16 +1127,10 @@ static void prueth_emac_restart(struct prueth *prueth)
>> icssg_set_port_state(emac1, ICSSG_EMAC_PORT_DISABLE);
>>
>> /* Stop both pru cores for both PRUeth ports*/
>> - prueth_emac_stop(emac0);
>> - prueth->emacs_initialized--;
>> - prueth_emac_stop(emac1);
>> - prueth->emacs_initialized--;
>> + prueth_emac_common_stop(prueth);
>>
>> /* Start both pru cores for both PRUeth ports */
>> - prueth_emac_start(prueth, emac0);
>> - prueth->emacs_initialized++;
>> - prueth_emac_start(prueth, emac1);
>> - prueth->emacs_initialized++;
>> + prueth_emac_common_start(prueth);
>>
>> /* Enable forwarding for both PRUeth ports */
>> icssg_set_port_state(emac0, ICSSG_EMAC_PORT_FORWARD);
>> @@ -1413,13 +1465,10 @@ static int prueth_probe(struct platform_device *pdev)
>> prueth->pa_stats = NULL;
>> }
>>
>> - if (eth0_node) {
>> + if (eth0_node || eth1_node) {
>> ret = prueth_get_cores(prueth, ICSS_SLICE0, false);
>> if (ret)
>> goto put_cores;
>> - }
>> -
>> - if (eth1_node) {
>> ret = prueth_get_cores(prueth, ICSS_SLICE1, false);
>> if (ret)
>> goto put_cores;
>> @@ -1618,14 +1667,12 @@ static int prueth_probe(struct platform_device *pdev)
>> pruss_put(prueth->pruss);
>>
>> put_cores:
>> - if (eth1_node) {
>> - prueth_put_cores(prueth, ICSS_SLICE1);
>> - of_node_put(eth1_node);
>> - }
>> -
>> - if (eth0_node) {
>> + if (eth0_node || eth1_node) {
>> prueth_put_cores(prueth, ICSS_SLICE0);
>> of_node_put(eth0_node);
>> +
>> + prueth_put_cores(prueth, ICSS_SLICE1);
>> + of_node_put(eth1_node);
>> }
>>
>> return ret;
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> index f5c1d473e9f9..b30f2e9a73d8 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> @@ -257,6 +257,7 @@ struct icssg_firmwares {
>> * @is_switchmode_supported: indicates platform support for switch mode
>> * @switch_id: ID for mapping switch ports to bridge
>> * @default_vlan: Default VLAN for host
>> + * @prus_running: flag to indicate if all pru cores are running
>> */
>> struct prueth {
>> struct device *dev;
>> @@ -298,6 +299,7 @@ struct prueth {
>> int default_vlan;
>> /** @vtbl_lock: Lock for vtbl in shared memory */
>> spinlock_t vtbl_lock;
>> + bool prus_running;
>> };
>>
>> struct emac_tx_ts_response {
>> @@ -361,6 +363,8 @@ int icssg_set_port_state(struct prueth_emac *emac,
>> enum icssg_port_state_cmd state);
>> void icssg_config_set_speed(struct prueth_emac *emac);
>> void icssg_config_half_duplex(struct prueth_emac *emac);
>> +void icssg_init_emac_mode(struct prueth *prueth);
>> +void icssg_init_fw_offload_mode(struct prueth *prueth);
>>
>> /* Buffer queue helpers */
>> int icssg_queue_pop(struct prueth *prueth, u8 queue);
>> @@ -377,6 +381,7 @@ void icssg_vtbl_modify(struct prueth_emac *emac, u8 vid, u8 port_mask,
>> u8 untag_mask, bool add);
>> u16 icssg_get_pvid(struct prueth_emac *emac);
>> void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port);
>> +int emac_fdb_flow_id_updated(struct prueth_emac *emac);
>> #define prueth_napi_to_tx_chn(pnapi) \
>> container_of(pnapi, struct prueth_tx_chn, napi_tx)
>>
>> --
>> 2.25.1
>>
>>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence.
2024-12-09 10:34 ` Meghana Malladi
@ 2024-12-09 12:39 ` Roger Quadros
2024-12-09 13:01 ` Meghana Malladi
2024-12-09 14:44 ` Diogo Ivo
1 sibling, 1 reply; 12+ messages in thread
From: Roger Quadros @ 2024-12-09 12:39 UTC (permalink / raw)
To: Meghana Malladi, vigneshr, jan.kiszka, javier.carrasco.cruz,
diogo.ivo, jacob.e.keller, horms, pabeni, kuba, edumazet, davem,
andrew+netdev
Cc: linux-kernel, netdev, linux-arm-kernel, srk, danishanwar
On 09/12/2024 12:34, Meghana Malladi wrote:
>
>
> On 05/12/24 18:38, Roger Quadros wrote:
>> Hi,
>>
>> On 05/12/2024 10:28, Meghana Malladi wrote:
>>> From: MD Danish Anwar <danishanwar@ti.com>
>>>
>>> Timesync related operations are ran in PRU0 cores for both ICSSG SLICE0
>>> and SLICE1. Currently whenever any ICSSG interface comes up we load the
>>> respective firmwares to PRU cores and whenever interface goes down, we
>>> stop the resective cores. Due to this, when SLICE0 goes down while
>>> SLICE1 is still active, PRU0 firmwares are unloaded and PRU0 core is
>>> stopped. This results in clock jump for SLICE1 interface as the timesync
>>> related operations are no longer running.
>>>
>>> As there are interdependencies between SLICE0 and SLICE1 firmwares,
>>> fix this by running both PRU0 and PRU1 firmwares as long as at least 1
>>> ICSSG interface is up. Add new flag in prueth struct to check if all
>>> firmwares are running.
>>>
>>> Use emacs_initialized as reference count to load the firmwares for the
>>> first and last interface up/down. Moving init_emac_mode and fw_offload_mode
>>> API outside of icssg_config to icssg_common_start API as they need
>>> to be called only once per firmware boot.
>>>
>>> Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
>>> ---
>>>
>>> Hi all,
>>>
>>> This patch is based on net-next tagged next-20241128.
>>> v2:https://lore.kernel.org/all/20241128122931.2494446-2-m-malladi@ti.com/
>>>
>>> * Changes since v2 (v3-v2):
>>> - error handling in caller function of prueth_emac_common_start()
>>> - Use prus_running flag check before stopping the firmwares
>>> Both suggested by Roger Quadros <rogerq@kernel.org>
>>>
>>> drivers/net/ethernet/ti/icssg/icssg_config.c | 45 ++++--
>>> drivers/net/ethernet/ti/icssg/icssg_config.h | 1 +
>>> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 157 ++++++++++++-------
>>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 5 +
>>> 4 files changed, 140 insertions(+), 68 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>> index 5d2491c2943a..342150756cf7 100644
>>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>> @@ -397,7 +397,7 @@ static int prueth_emac_buffer_setup(struct prueth_emac *emac)
>>> return 0;
>>> }
>>>
> [ ... ]
>
>>> +static int prueth_emac_common_start(struct prueth *prueth)
>>> +{
>>> + struct prueth_emac *emac;
>>> + int ret = 0;
>>> + int slice;
>>> +
>>> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
>>> + return -EINVAL;
>>> +
>>> + /* clear SMEM and MSMC settings for all slices */
>>> + memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
>>> + memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
>>> +
>>> + icssg_class_default(prueth->miig_rt, ICSS_SLICE0, 0, false);
>>> + icssg_class_default(prueth->miig_rt, ICSS_SLICE1, 0, false);
>>> +
>>> + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
>>> + icssg_init_fw_offload_mode(prueth);
>>> + else
>>> + icssg_init_emac_mode(prueth);
>>> +
>>> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
>>> + emac = prueth->emac[slice];
>>> + if (emac) {
>>> + ret |= icssg_config(prueth, emac, slice);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> + ret |= prueth_emac_start(prueth, slice);
>>> + }
>>
>> need newline?
>>
>
> Yes I will add it.
>
>>> + if (!ret)
>>> + prueth->prus_running = 1;
>>> + else
>>> + return ret;
>>> +
>>> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
>>> + prueth->emac[ICSS_SLICE1];
>>> + ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
>>> + emac, IEP_DEFAULT_CYCLE_TIME_NS);
>>> + if (ret) {
>>> + dev_err(prueth->dev, "Failed to initialize IEP module\n");
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int prueth_emac_common_stop(struct prueth *prueth)
>>> +{
>>> + struct prueth_emac *emac;
>>> + int slice;
>>> +
>>> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
>>> + return -EINVAL;
>>> +
>>> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE0);
>>> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE1);
>>> +
>>> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
>>> + if (prueth->prus_running) {
>>> + rproc_shutdown(prueth->txpru[slice]);
>>> + rproc_shutdown(prueth->rtu[slice]);
>>> + rproc_shutdown(prueth->pru[slice]);
>>> + }
>>> + }
>>> + prueth->prus_running = 0;
>>> +
>>> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
>>> + prueth->emac[ICSS_SLICE1];
>>> + icss_iep_exit(emac->iep);
>>
>> if icss_iep_init() failed at prueth_emac_common_start(), we should not be
>> calling icss_iep_exit(). Maybe you need another flag for iep_init status?
>>
>
> Yes I have thought of it as well. In icss_iep_init() does lot of iep register configuration and in the end it enables iep by setting IEP_CNT_ENABLE bit and ptp_clock_register(). Whereas in icss_iep_exit() it checks for ptp_clock and pps/perout. And calls icss_iep_disable() which again clears IEP_CNT_ENABLE.
>
> So I see no harm in calling icss_iep_exit() even if icss_iep_init() as it overwrites the existing configuration only. But if you think this doesn't look good I can definitely add new flag for iep as well. But IMO I think this flag would be redundant, please correct me if I am wrong. So which one sounds better?
But isn't icss_iep_exit() also calling ptp_clock_unregister()?
If it is safe to do that even in error condition then it doesn't matter.
>
>> Is it better to call icss_iep_exit() at the top before icssg_class_disable()?
>>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> /* called back by PHY layer if there is change in link state of hw port*/
>>> static void emac_adjust_link(struct net_device *ndev)
>>> {
>>> @@ -369,12 +432,13 @@ static void prueth_iep_settime(void *clockops_data, u64 ns)
>>> {
>>> struct icssg_setclock_desc __iomem *sc_descp;
>>> struct prueth_emac *emac = clockops_data;
>>> + struct prueth *prueth = emac->prueth;
>>> struct icssg_setclock_desc sc_desc;
>>> u64 cyclecount;
>>> u32 cycletime;
>>> int timeout;
>>> - if (!emac->fw_running)
>>> + if (!prueth->prus_running)
>>> return;
>>> sc_descp = emac->prueth->shram.va + TIMESYNC_FW_WC_SETCLOCK_DESC_OFFSET;
>>> @@ -543,23 +607,17 @@ static int emac_ndo_open(struct net_device *ndev)
>>> {
>>> struct prueth_emac *emac = netdev_priv(ndev);
>>> int ret, i, num_data_chn = emac->tx_ch_num;
>>> + struct icssg_flow_cfg __iomem *flow_cfg;
>>> struct prueth *prueth = emac->prueth;
>>> int slice = prueth_emac_slice(emac);
>>> struct device *dev = prueth->dev;
>>> int max_rx_flows;
>>> int rx_flow;
>>> - /* clear SMEM and MSMC settings for all slices */
>>> - if (!prueth->emacs_initialized) {
>>> - memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
>>> - memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
>>> - }
>>> -
>>> /* set h/w MAC as user might have re-configured */
>>> ether_addr_copy(emac->mac_addr, ndev->dev_addr);
>>> icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>>> - icssg_class_default(prueth->miig_rt, slice, 0, false);
>>> icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>>> /* Notify the stack of the actual queue counts. */
>>> @@ -597,18 +655,23 @@ static int emac_ndo_open(struct net_device *ndev)
>>> goto cleanup_napi;
>>> }
>>> - /* reset and start PRU firmware */
>>> - ret = prueth_emac_start(prueth, emac);
>>> - if (ret)
>>> - goto free_rx_irq;
>>> + if (!prueth->emacs_initialized) {
>>> + ret = prueth_emac_common_start(prueth);
>>> + if (ret)
>>> + goto stop;
>>> + }
>>> - icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
>>> + flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
>>> + writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);
>>> + ret = emac_fdb_flow_id_updated(emac);
>>> - if (!prueth->emacs_initialized) {
>>> - ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
>>> - emac, IEP_DEFAULT_CYCLE_TIME_NS);
>>> + if (ret) {
>>> + netdev_err(ndev, "Failed to update Rx Flow ID %d", ret);
>>> + goto stop;
>>> }
>>> + icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
>>> +
>>> ret = request_threaded_irq(emac->tx_ts_irq, NULL, prueth_tx_ts_irq,
>>> IRQF_ONESHOT, dev_name(dev), emac);
>>> if (ret)
>>> @@ -653,8 +716,7 @@ static int emac_ndo_open(struct net_device *ndev)
>>> free_tx_ts_irq:
>>> free_irq(emac->tx_ts_irq, emac);
>>> stop:
>>> - prueth_emac_stop(emac);
>>> -free_rx_irq:
>>> + prueth_emac_common_stop(prueth);
>>> free_irq(emac->rx_chns.irq[rx_flow], emac);
>>> cleanup_napi:
>>> prueth_ndev_del_tx_napi(emac, emac->tx_ch_num);
>>> @@ -689,8 +751,6 @@ static int emac_ndo_stop(struct net_device *ndev)
>>> if (ndev->phydev)
>>> phy_stop(ndev->phydev);
>>> - icssg_class_disable(prueth->miig_rt, prueth_emac_slice(emac));
>>> -
>>> if (emac->prueth->is_hsr_offload_mode)
>>> __dev_mc_unsync(ndev, icssg_prueth_hsr_del_mcast);
>>> else
>>> @@ -728,11 +788,9 @@ static int emac_ndo_stop(struct net_device *ndev)
>>> /* Destroying the queued work in ndo_stop() */
>>> cancel_delayed_work_sync(&emac->stats_work);
>>> - if (prueth->emacs_initialized == 1)
>>> - icss_iep_exit(emac->iep);
>>> -
>>> /* stop PRUs */
>>> - prueth_emac_stop(emac);
>>> + if (prueth->emacs_initialized == 1)
>>> + prueth_emac_common_stop(prueth);
>>> free_irq(emac->tx_ts_irq, emac);
>>> @@ -1069,16 +1127,10 @@ static void prueth_emac_restart(struct prueth *prueth)
>>> icssg_set_port_state(emac1, ICSSG_EMAC_PORT_DISABLE);
>>> /* Stop both pru cores for both PRUeth ports*/
>>> - prueth_emac_stop(emac0);
>>> - prueth->emacs_initialized--;
>>> - prueth_emac_stop(emac1);
>>> - prueth->emacs_initialized--;
>>> + prueth_emac_common_stop(prueth);
>>> /* Start both pru cores for both PRUeth ports */
>>> - prueth_emac_start(prueth, emac0);
>>> - prueth->emacs_initialized++;
>>> - prueth_emac_start(prueth, emac1);
>>> - prueth->emacs_initialized++;
>>> + prueth_emac_common_start(prueth);
>>
>> But this can fail? You need to deal with failure condition appropriately.
>>
>
> I haven't added failure conditions for two reasons:
> - Existing code also didn't have any error checks
> - This func simply reloads a new firmware, given everything is already working with the old one.
>
> I can still handle error cases by changing this func to return int (currently it is void) and caller of the functions should print error and immediately return. Thoughts on this?
At least an error message somewhere will help to debug later.
>
>>> /* Enable forwarding for both PRUeth ports */
>>> icssg_set_port_state(emac0, ICSSG_EMAC_PORT_FORWARD);
>>> @@ -1413,13 +1465,10 @@ static int prueth_probe(struct platform_device *pdev)
>>> prueth->pa_stats = NULL;
>>> }
>>> - if (eth0_node) {
>>> + if (eth0_node || eth1_node) {
>>> ret = prueth_get_cores(prueth, ICSS_SLICE0, false);
>>> if (ret)
>>> goto put_cores;
>>> - }
>>> -
>>> - if (eth1_node) {
>>> ret = prueth_get_cores(prueth, ICSS_SLICE1, false);
>>> if (ret)
>>> goto put_cores;
>>> @@ -1618,14 +1667,12 @@ static int prueth_probe(struct platform_device *pdev)
>>> pruss_put(prueth->pruss);
>>> put_cores:
>>> - if (eth1_node) {
>>> - prueth_put_cores(prueth, ICSS_SLICE1);
>>> - of_node_put(eth1_node);
>>> - }
>>> -
>>> - if (eth0_node) {
>>> + if (eth0_node || eth1_node) {
>>> prueth_put_cores(prueth, ICSS_SLICE0);
>>> of_node_put(eth0_node);
>>> +
>>> + prueth_put_cores(prueth, ICSS_SLICE1);
>>> + of_node_put(eth1_node);
>>> }
>>> return ret;
>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>> index f5c1d473e9f9..b30f2e9a73d8 100644
>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>> @@ -257,6 +257,7 @@ struct icssg_firmwares {
>>> * @is_switchmode_supported: indicates platform support for switch mode
>>> * @switch_id: ID for mapping switch ports to bridge
>>> * @default_vlan: Default VLAN for host
>>> + * @prus_running: flag to indicate if all pru cores are running
>>> */
>>> struct prueth {
>>> struct device *dev;
>>> @@ -298,6 +299,7 @@ struct prueth {
>>> int default_vlan;
>>> /** @vtbl_lock: Lock for vtbl in shared memory */
>>> spinlock_t vtbl_lock;
>>> + bool prus_running;
>>
>> I think you don't need fw_running flag anymore. Could you please remove it
>> from struct prueth_emac?
>>
>
> This flag is still being used by SR1, for which this patch doesn't apply. So I prefer not touching this flag for the sake of SR1.
ok let's leave it there then.
>
>>> };
>>> struct emac_tx_ts_response {
>>> @@ -361,6 +363,8 @@ int icssg_set_port_state(struct prueth_emac *emac,
>>> enum icssg_port_state_cmd state);
>>> void icssg_config_set_speed(struct prueth_emac *emac);
>>> void icssg_config_half_duplex(struct prueth_emac *emac);
>>> +void icssg_init_emac_mode(struct prueth *prueth);
>>> +void icssg_init_fw_offload_mode(struct prueth *prueth);
>>> /* Buffer queue helpers */
>>> int icssg_queue_pop(struct prueth *prueth, u8 queue);
>>> @@ -377,6 +381,7 @@ void icssg_vtbl_modify(struct prueth_emac *emac, u8 vid, u8 port_mask,
>>> u8 untag_mask, bool add);
>>> u16 icssg_get_pvid(struct prueth_emac *emac);
>>> void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port);
>>> +int emac_fdb_flow_id_updated(struct prueth_emac *emac);
>>> #define prueth_napi_to_tx_chn(pnapi) \
>>> container_of(pnapi, struct prueth_tx_chn, napi_tx)
>>>
>>
--
cheers,
-roger
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence.
2024-12-09 12:39 ` Roger Quadros
@ 2024-12-09 13:01 ` Meghana Malladi
0 siblings, 0 replies; 12+ messages in thread
From: Meghana Malladi @ 2024-12-09 13:01 UTC (permalink / raw)
To: Roger Quadros, vigneshr, jan.kiszka, javier.carrasco.cruz,
diogo.ivo, jacob.e.keller, horms, pabeni, kuba, edumazet, davem,
andrew+netdev
Cc: linux-kernel, netdev, linux-arm-kernel, srk, danishanwar
On 09/12/24 18:09, Roger Quadros wrote:
>
>
> On 09/12/2024 12:34, Meghana Malladi wrote:
>>
>>
>> On 05/12/24 18:38, Roger Quadros wrote:
>>> Hi,
>>>
>>> On 05/12/2024 10:28, Meghana Malladi wrote:
>>>> From: MD Danish Anwar <danishanwar@ti.com>
>>>>
>>>> Timesync related operations are ran in PRU0 cores for both ICSSG SLICE0
>>>> and SLICE1. Currently whenever any ICSSG interface comes up we load the
>>>> respective firmwares to PRU cores and whenever interface goes down, we
>>>> stop the resective cores. Due to this, when SLICE0 goes down while
>>>> SLICE1 is still active, PRU0 firmwares are unloaded and PRU0 core is
>>>> stopped. This results in clock jump for SLICE1 interface as the timesync
>>>> related operations are no longer running.
>>>>
>>>> As there are interdependencies between SLICE0 and SLICE1 firmwares,
>>>> fix this by running both PRU0 and PRU1 firmwares as long as at least 1
>>>> ICSSG interface is up. Add new flag in prueth struct to check if all
>>>> firmwares are running.
>>>>
>>>> Use emacs_initialized as reference count to load the firmwares for the
>>>> first and last interface up/down. Moving init_emac_mode and fw_offload_mode
>>>> API outside of icssg_config to icssg_common_start API as they need
>>>> to be called only once per firmware boot.
>>>>
>>>> Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
>>>> ---
>>>>
>>>> Hi all,
>>>>
>>>> This patch is based on net-next tagged next-20241128.
>>>> v2:https://lore.kernel.org/all/20241128122931.2494446-2-m-malladi@ti.com/
>>>>
>>>> * Changes since v2 (v3-v2):
>>>> - error handling in caller function of prueth_emac_common_start()
>>>> - Use prus_running flag check before stopping the firmwares
>>>> Both suggested by Roger Quadros <rogerq@kernel.org>
>>>>
>>>> drivers/net/ethernet/ti/icssg/icssg_config.c | 45 ++++--
>>>> drivers/net/ethernet/ti/icssg/icssg_config.h | 1 +
>>>> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 157 ++++++++++++-------
>>>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 5 +
>>>> 4 files changed, 140 insertions(+), 68 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>>> index 5d2491c2943a..342150756cf7 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>>> @@ -397,7 +397,7 @@ static int prueth_emac_buffer_setup(struct prueth_emac *emac)
>>>> return 0;
>>>> }
>>>>
>> [ ... ]
>>
>>>> +static int prueth_emac_common_start(struct prueth *prueth)
>>>> +{
>>>> + struct prueth_emac *emac;
>>>> + int ret = 0;
>>>> + int slice;
>>>> +
>>>> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
>>>> + return -EINVAL;
>>>> +
>>>> + /* clear SMEM and MSMC settings for all slices */
>>>> + memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
>>>> + memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
>>>> +
>>>> + icssg_class_default(prueth->miig_rt, ICSS_SLICE0, 0, false);
>>>> + icssg_class_default(prueth->miig_rt, ICSS_SLICE1, 0, false);
>>>> +
>>>> + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
>>>> + icssg_init_fw_offload_mode(prueth);
>>>> + else
>>>> + icssg_init_emac_mode(prueth);
>>>> +
>>>> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
>>>> + emac = prueth->emac[slice];
>>>> + if (emac) {
>>>> + ret |= icssg_config(prueth, emac, slice);
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>>>> + ret |= prueth_emac_start(prueth, slice);
>>>> + }
>>>
>>> need newline?
>>>
>>
>> Yes I will add it.
>>
>>>> + if (!ret)
>>>> + prueth->prus_running = 1;
>>>> + else
>>>> + return ret;
>>>> +
>>>> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
>>>> + prueth->emac[ICSS_SLICE1];
>>>> + ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
>>>> + emac, IEP_DEFAULT_CYCLE_TIME_NS);
>>>> + if (ret) {
>>>> + dev_err(prueth->dev, "Failed to initialize IEP module\n");
>>>> + return ret;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int prueth_emac_common_stop(struct prueth *prueth)
>>>> +{
>>>> + struct prueth_emac *emac;
>>>> + int slice;
>>>> +
>>>> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
>>>> + return -EINVAL;
>>>> +
>>>> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE0);
>>>> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE1);
>>>> +
>>>> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
>>>> + if (prueth->prus_running) {
>>>> + rproc_shutdown(prueth->txpru[slice]);
>>>> + rproc_shutdown(prueth->rtu[slice]);
>>>> + rproc_shutdown(prueth->pru[slice]);
>>>> + }
>>>> + }
>>>> + prueth->prus_running = 0;
>>>> +
>>>> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
>>>> + prueth->emac[ICSS_SLICE1];
>>>> + icss_iep_exit(emac->iep);
>>>
>>> if icss_iep_init() failed at prueth_emac_common_start(), we should not be
>>> calling icss_iep_exit(). Maybe you need another flag for iep_init status?
>>>
>>
>> Yes I have thought of it as well. In icss_iep_init() does lot of iep register configuration and in the end it enables iep by setting IEP_CNT_ENABLE bit and ptp_clock_register(). Whereas in icss_iep_exit() it checks for ptp_clock and pps/perout. And calls icss_iep_disable() which again clears IEP_CNT_ENABLE.
>>
>> So I see no harm in calling icss_iep_exit() even if icss_iep_init() as it overwrites the existing configuration only. But if you think this doesn't look good I can definitely add new flag for iep as well. But IMO I think this flag would be redundant, please correct me if I am wrong. So which one sounds better?
>
> But isn't icss_iep_exit() also calling ptp_clock_unregister()?
> If it is safe to do that even in error condition then it doesn't matter.
>
It checks for iep->ptp_clock before, only if it is not NULL it will
unregister, so yeah it is safe.
>>
>>> Is it better to call icss_iep_exit() at the top before icssg_class_disable()?
>>>
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> /* called back by PHY layer if there is change in link state of hw port*/
>>>> static void emac_adjust_link(struct net_device *ndev)
>>>> {
>>>> @@ -369,12 +432,13 @@ static void prueth_iep_settime(void *clockops_data, u64 ns)
>>>> {
>>>> struct icssg_setclock_desc __iomem *sc_descp;
>>>> struct prueth_emac *emac = clockops_data;
>>>> + struct prueth *prueth = emac->prueth;
>>>> struct icssg_setclock_desc sc_desc;
>>>> u64 cyclecount;
>>>> u32 cycletime;
>>>> int timeout;
>>>> - if (!emac->fw_running)
>>>> + if (!prueth->prus_running)
>>>> return;
>>>> sc_descp = emac->prueth->shram.va + TIMESYNC_FW_WC_SETCLOCK_DESC_OFFSET;
>>>> @@ -543,23 +607,17 @@ static int emac_ndo_open(struct net_device *ndev)
>>>> {
>>>> struct prueth_emac *emac = netdev_priv(ndev);
>>>> int ret, i, num_data_chn = emac->tx_ch_num;
>>>> + struct icssg_flow_cfg __iomem *flow_cfg;
>>>> struct prueth *prueth = emac->prueth;
>>>> int slice = prueth_emac_slice(emac);
>>>> struct device *dev = prueth->dev;
>>>> int max_rx_flows;
>>>> int rx_flow;
>>>> - /* clear SMEM and MSMC settings for all slices */
>>>> - if (!prueth->emacs_initialized) {
>>>> - memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
>>>> - memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
>>>> - }
>>>> -
>>>> /* set h/w MAC as user might have re-configured */
>>>> ether_addr_copy(emac->mac_addr, ndev->dev_addr);
>>>> icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>>>> - icssg_class_default(prueth->miig_rt, slice, 0, false);
>>>> icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>>>> /* Notify the stack of the actual queue counts. */
>>>> @@ -597,18 +655,23 @@ static int emac_ndo_open(struct net_device *ndev)
>>>> goto cleanup_napi;
>>>> }
>>>> - /* reset and start PRU firmware */
>>>> - ret = prueth_emac_start(prueth, emac);
>>>> - if (ret)
>>>> - goto free_rx_irq;
>>>> + if (!prueth->emacs_initialized) {
>>>> + ret = prueth_emac_common_start(prueth);
>>>> + if (ret)
>>>> + goto stop;
>>>> + }
>>>> - icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
>>>> + flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
>>>> + writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);
>>>> + ret = emac_fdb_flow_id_updated(emac);
>>>> - if (!prueth->emacs_initialized) {
>>>> - ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
>>>> - emac, IEP_DEFAULT_CYCLE_TIME_NS);
>>>> + if (ret) {
>>>> + netdev_err(ndev, "Failed to update Rx Flow ID %d", ret);
>>>> + goto stop;
>>>> }
>>>> + icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
>>>> +
>>>> ret = request_threaded_irq(emac->tx_ts_irq, NULL, prueth_tx_ts_irq,
>>>> IRQF_ONESHOT, dev_name(dev), emac);
>>>> if (ret)
>>>> @@ -653,8 +716,7 @@ static int emac_ndo_open(struct net_device *ndev)
>>>> free_tx_ts_irq:
>>>> free_irq(emac->tx_ts_irq, emac);
>>>> stop:
>>>> - prueth_emac_stop(emac);
>>>> -free_rx_irq:
>>>> + prueth_emac_common_stop(prueth);
>>>> free_irq(emac->rx_chns.irq[rx_flow], emac);
>>>> cleanup_napi:
>>>> prueth_ndev_del_tx_napi(emac, emac->tx_ch_num);
>>>> @@ -689,8 +751,6 @@ static int emac_ndo_stop(struct net_device *ndev)
>>>> if (ndev->phydev)
>>>> phy_stop(ndev->phydev);
>>>> - icssg_class_disable(prueth->miig_rt, prueth_emac_slice(emac));
>>>> -
>>>> if (emac->prueth->is_hsr_offload_mode)
>>>> __dev_mc_unsync(ndev, icssg_prueth_hsr_del_mcast);
>>>> else
>>>> @@ -728,11 +788,9 @@ static int emac_ndo_stop(struct net_device *ndev)
>>>> /* Destroying the queued work in ndo_stop() */
>>>> cancel_delayed_work_sync(&emac->stats_work);
>>>> - if (prueth->emacs_initialized == 1)
>>>> - icss_iep_exit(emac->iep);
>>>> -
>>>> /* stop PRUs */
>>>> - prueth_emac_stop(emac);
>>>> + if (prueth->emacs_initialized == 1)
>>>> + prueth_emac_common_stop(prueth);
>>>> free_irq(emac->tx_ts_irq, emac);
>>>> @@ -1069,16 +1127,10 @@ static void prueth_emac_restart(struct prueth *prueth)
>>>> icssg_set_port_state(emac1, ICSSG_EMAC_PORT_DISABLE);
>>>> /* Stop both pru cores for both PRUeth ports*/
>>>> - prueth_emac_stop(emac0);
>>>> - prueth->emacs_initialized--;
>>>> - prueth_emac_stop(emac1);
>>>> - prueth->emacs_initialized--;
>>>> + prueth_emac_common_stop(prueth);
>>>> /* Start both pru cores for both PRUeth ports */
>>>> - prueth_emac_start(prueth, emac0);
>>>> - prueth->emacs_initialized++;
>>>> - prueth_emac_start(prueth, emac1);
>>>> - prueth->emacs_initialized++;
>>>> + prueth_emac_common_start(prueth);
>>>
>>> But this can fail? You need to deal with failure condition appropriately.
>>>
>>
>> I haven't added failure conditions for two reasons:
>> - Existing code also didn't have any error checks
>> - This func simply reloads a new firmware, given everything is already working with the old one.
>>
>> I can still handle error cases by changing this func to return int (currently it is void) and caller of the functions should print error and immediately return. Thoughts on this?
>
> At least an error message somewhere will help to debug later.
>
Ok sure, I will add it then.
>>
>>>> /* Enable forwarding for both PRUeth ports */
>>>> icssg_set_port_state(emac0, ICSSG_EMAC_PORT_FORWARD);
>>>> @@ -1413,13 +1465,10 @@ static int prueth_probe(struct platform_device *pdev)
>>>> prueth->pa_stats = NULL;
>>>> }
>>>> - if (eth0_node) {
>>>> + if (eth0_node || eth1_node) {
>>>> ret = prueth_get_cores(prueth, ICSS_SLICE0, false);
>>>> if (ret)
>>>> goto put_cores;
>>>> - }
>>>> -
>>>> - if (eth1_node) {
>>>> ret = prueth_get_cores(prueth, ICSS_SLICE1, false);
>>>> if (ret)
>>>> goto put_cores;
>>>> @@ -1618,14 +1667,12 @@ static int prueth_probe(struct platform_device *pdev)
>>>> pruss_put(prueth->pruss);
>>>> put_cores:
>>>> - if (eth1_node) {
>>>> - prueth_put_cores(prueth, ICSS_SLICE1);
>>>> - of_node_put(eth1_node);
>>>> - }
>>>> -
>>>> - if (eth0_node) {
>>>> + if (eth0_node || eth1_node) {
>>>> prueth_put_cores(prueth, ICSS_SLICE0);
>>>> of_node_put(eth0_node);
>>>> +
>>>> + prueth_put_cores(prueth, ICSS_SLICE1);
>>>> + of_node_put(eth1_node);
>>>> }
>>>> return ret;
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> index f5c1d473e9f9..b30f2e9a73d8 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> @@ -257,6 +257,7 @@ struct icssg_firmwares {
>>>> * @is_switchmode_supported: indicates platform support for switch mode
>>>> * @switch_id: ID for mapping switch ports to bridge
>>>> * @default_vlan: Default VLAN for host
>>>> + * @prus_running: flag to indicate if all pru cores are running
>>>> */
>>>> struct prueth {
>>>> struct device *dev;
>>>> @@ -298,6 +299,7 @@ struct prueth {
>>>> int default_vlan;
>>>> /** @vtbl_lock: Lock for vtbl in shared memory */
>>>> spinlock_t vtbl_lock;
>>>> + bool prus_running;
>>>
>>> I think you don't need fw_running flag anymore. Could you please remove it
>>> from struct prueth_emac?
>>>
>>
>> This flag is still being used by SR1, for which this patch doesn't apply. So I prefer not touching this flag for the sake of SR1.
>
> ok let's leave it there then.
>
>>
>>>> };
>>>> struct emac_tx_ts_response {
>>>> @@ -361,6 +363,8 @@ int icssg_set_port_state(struct prueth_emac *emac,
>>>> enum icssg_port_state_cmd state);
>>>> void icssg_config_set_speed(struct prueth_emac *emac);
>>>> void icssg_config_half_duplex(struct prueth_emac *emac);
>>>> +void icssg_init_emac_mode(struct prueth *prueth);
>>>> +void icssg_init_fw_offload_mode(struct prueth *prueth);
>>>> /* Buffer queue helpers */
>>>> int icssg_queue_pop(struct prueth *prueth, u8 queue);
>>>> @@ -377,6 +381,7 @@ void icssg_vtbl_modify(struct prueth_emac *emac, u8 vid, u8 port_mask,
>>>> u8 untag_mask, bool add);
>>>> u16 icssg_get_pvid(struct prueth_emac *emac);
>>>> void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port);
>>>> +int emac_fdb_flow_id_updated(struct prueth_emac *emac);
>>>> #define prueth_napi_to_tx_chn(pnapi) \
>>>> container_of(pnapi, struct prueth_tx_chn, napi_tx)
>>>>
>>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence.
2024-12-09 10:34 ` Meghana Malladi
2024-12-09 12:39 ` Roger Quadros
@ 2024-12-09 14:44 ` Diogo Ivo
1 sibling, 0 replies; 12+ messages in thread
From: Diogo Ivo @ 2024-12-09 14:44 UTC (permalink / raw)
To: Meghana Malladi, Roger Quadros, vigneshr, jan.kiszka,
javier.carrasco.cruz, jacob.e.keller, horms, pabeni, kuba,
edumazet, davem, andrew+netdev
Cc: linux-kernel, netdev, linux-arm-kernel, srk, danishanwar
Hi all,
On 12/9/24 10:34 AM, Meghana Malladi wrote:
>
>
> On 05/12/24 18:38, Roger Quadros wrote:
>> Hi,
>>
>> On 05/12/2024 10:28, Meghana Malladi wrote:
>>> From: MD Danish Anwar <danishanwar@ti.com>
>>>
>>> Timesync related operations are ran in PRU0 cores for both ICSSG SLICE0
>>> and SLICE1. Currently whenever any ICSSG interface comes up we load the
>>> respective firmwares to PRU cores and whenever interface goes down, we
>>> stop the resective cores. Due to this, when SLICE0 goes down while
>>> SLICE1 is still active, PRU0 firmwares are unloaded and PRU0 core is
>>> stopped. This results in clock jump for SLICE1 interface as the timesync
>>> related operations are no longer running.
>>>
>>> As there are interdependencies between SLICE0 and SLICE1 firmwares,
>>> fix this by running both PRU0 and PRU1 firmwares as long as at least 1
>>> ICSSG interface is up. Add new flag in prueth struct to check if all
>>> firmwares are running.
>>>
>>> Use emacs_initialized as reference count to load the firmwares for the
>>> first and last interface up/down. Moving init_emac_mode and
>>> fw_offload_mode
>>> API outside of icssg_config to icssg_common_start API as they need
>>> to be called only once per firmware boot.
>>>
>>> Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
>>> ---
>>>
>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/
>>> net/ethernet/ti/icssg/icssg_prueth.h
>>> index f5c1d473e9f9..b30f2e9a73d8 100644
>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>> @@ -257,6 +257,7 @@ struct icssg_firmwares {
>>> * @is_switchmode_supported: indicates platform support for switch
>>> mode
>>> * @switch_id: ID for mapping switch ports to bridge
>>> * @default_vlan: Default VLAN for host
>>> + * @prus_running: flag to indicate if all pru cores are running
>>> */
>>> struct prueth {
>>> struct device *dev;
>>> @@ -298,6 +299,7 @@ struct prueth {
>>> int default_vlan;
>>> /** @vtbl_lock: Lock for vtbl in shared memory */
>>> spinlock_t vtbl_lock;
>>> + bool prus_running;
>>
>> I think you don't need fw_running flag anymore. Could you please
>> remove it
>> from struct prueth_emac?
>>
>
> This flag is still being used by SR1, for which this patch doesn't
> apply. So I prefer not touching this flag for the sake of SR1.
Currently for SR1.0 this flag is set but not used anywhere in the code,
so it can be removed.
Best regards,
Diogo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence.
2024-12-05 8:24 [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence Meghana Malladi
@ 2024-12-05 8:30 ` Meghana Malladi
0 siblings, 0 replies; 12+ messages in thread
From: Meghana Malladi @ 2024-12-05 8:30 UTC (permalink / raw)
To: vigneshr, Roger Quadros, javier.carrasco.cruz, diogo.ivo, horms,
pabeni, kuba, edumazet, davem, andrew+netdev
Cc: linux-kernel, netdev, linux-arm-kernel, srk, danishanwar
Hi All,
Apologies for this mishap (incorrect patch series). Please ignore this
patch.
On 05/12/24 13:54, Meghana Malladi wrote:
> From: MD Danish Anwar <danishanwar@ti.com>
>
> Timesync related operations are ran in PRU0 cores for both ICSSG SLICE0
> and SLICE1. Currently whenever any ICSSG interface comes up we load the
> respective firmwares to PRU cores and whenever interface goes down, we
> stop the resective cores. Due to this, when SLICE0 goes down while
> SLICE1 is still active, PRU0 firmwares are unloaded and PRU0 core is
> stopped. This results in clock jump for SLICE1 interface as the timesync
> related operations are no longer running.
>
> As there are interdependencies between SLICE0 and SLICE1 firmwares,
> fix this by running both PRU0 and PRU1 firmwares as long as at least 1
> ICSSG interface is up. Add new flag in prueth struct to check if all
> firmwares are running.
>
> Use emacs_initialized as reference count to load the firmwares for the
> first and last interface up/down. Moving init_emac_mode and fw_offload_mode
> API outside of icssg_config to icssg_common_start API as they need
> to be called only once per firmware boot.
>
> Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
> ---
>
> Hi all,
>
> This patch is based on net-next tagged next-20241128.
> v2:https://lore.kernel.org/all/20241128122931.2494446-2-m-malladi@ti.com/
>
> * Changes since v2 (v3-v2):
> - error handling in caller function of prueth_emac_common_start()
> - Use prus_running flag check before stopping the firmwares
> Both suggested by Roger Quadros <rogerq@kernel.org>
>
> drivers/net/ethernet/ti/icssg/icssg_config.c | 45 ++++--
> drivers/net/ethernet/ti/icssg/icssg_config.h | 1 +
> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 157 ++++++++++++-------
> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 5 +
> 4 files changed, 140 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
> index 5d2491c2943a..342150756cf7 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
> @@ -397,7 +397,7 @@ static int prueth_emac_buffer_setup(struct prueth_emac *emac)
> return 0;
> }
>
> -static void icssg_init_emac_mode(struct prueth *prueth)
> +void icssg_init_emac_mode(struct prueth *prueth)
> {
> /* When the device is configured as a bridge and it is being brought
> * back to the emac mode, the host mac address has to be set as 0.
> @@ -406,9 +406,6 @@ static void icssg_init_emac_mode(struct prueth *prueth)
> int i;
> u8 mac[ETH_ALEN] = { 0 };
>
> - 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);
> @@ -423,15 +420,13 @@ static void icssg_init_emac_mode(struct prueth *prueth)
> /* Clear host MAC address */
> icssg_class_set_host_mac_addr(prueth->miig_rt, mac);
> }
> +EXPORT_SYMBOL_GPL(icssg_init_emac_mode);
>
> -static void icssg_init_fw_offload_mode(struct prueth *prueth)
> +void icssg_init_fw_offload_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);
> @@ -448,6 +443,7 @@ static void icssg_init_fw_offload_mode(struct prueth *prueth)
> 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);
> }
> +EXPORT_SYMBOL_GPL(icssg_init_fw_offload_mode);
>
> int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
> {
> @@ -455,11 +451,6 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
> struct icssg_flow_cfg __iomem *flow_cfg;
> int ret;
>
> - if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
> - icssg_init_fw_offload_mode(prueth);
> - else
> - icssg_init_emac_mode(prueth);
> -
> memset_io(config, 0, TAS_GATE_MASK_LIST0);
> icssg_miig_queues_init(prueth, slice);
>
> @@ -786,3 +777,31 @@ void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port)
> writel(pvid, prueth->shram.va + EMAC_ICSSG_SWITCH_PORT0_DEFAULT_VLAN_OFFSET);
> }
> EXPORT_SYMBOL_GPL(icssg_set_pvid);
> +
> +int emac_fdb_flow_id_updated(struct prueth_emac *emac)
> +{
> + struct mgmt_cmd_rsp fdb_cmd_rsp = { 0 };
> + int slice = prueth_emac_slice(emac);
> + struct mgmt_cmd fdb_cmd = { 0 };
> + int ret = 0;
> +
> + fdb_cmd.header = ICSSG_FW_MGMT_CMD_HEADER;
> + fdb_cmd.type = ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW;
> + fdb_cmd.seqnum = ++(emac->prueth->icssg_hwcmdseq);
> + fdb_cmd.param = 0;
> +
> + fdb_cmd.param |= (slice << 4);
> + fdb_cmd.cmd_args[0] = 0;
> +
> + ret = icssg_send_fdb_msg(emac, &fdb_cmd, &fdb_cmd_rsp);
> +
> + if (ret)
> + return ret;
> +
> + WARN_ON(fdb_cmd.seqnum != fdb_cmd_rsp.seqnum);
> + if (fdb_cmd_rsp.status == 1)
> + return 0;
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(emac_fdb_flow_id_updated);
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
> index 92c2deaa3068..c884e9fa099e 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
> @@ -55,6 +55,7 @@ struct icssg_rxq_ctx {
> #define ICSSG_FW_MGMT_FDB_CMD_TYPE 0x03
> #define ICSSG_FW_MGMT_CMD_TYPE 0x04
> #define ICSSG_FW_MGMT_PKT 0x80000000
> +#define ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW 0x05
>
> struct icssg_r30_cmd {
> u32 cmd[4];
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index c568c84a032b..2e22e793b01a 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -164,11 +164,11 @@ static struct icssg_firmwares icssg_emac_firmwares[] = {
> }
> };
>
> -static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> +static int prueth_emac_start(struct prueth *prueth, int slice)
> {
> struct icssg_firmwares *firmwares;
> struct device *dev = prueth->dev;
> - int slice, ret;
> + int ret;
>
> if (prueth->is_switch_mode)
> firmwares = icssg_switch_firmwares;
> @@ -177,16 +177,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> else
> firmwares = icssg_emac_firmwares;
>
> - slice = prueth_emac_slice(emac);
> - if (slice < 0) {
> - netdev_err(emac->ndev, "invalid port\n");
> - return -EINVAL;
> - }
> -
> - ret = icssg_config(prueth, emac, slice);
> - if (ret)
> - return ret;
> -
> ret = rproc_set_firmware(prueth->pru[slice], firmwares[slice].pru);
> ret = rproc_boot(prueth->pru[slice]);
> if (ret) {
> @@ -208,7 +198,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> goto halt_rtu;
> }
>
> - emac->fw_running = 1;
> return 0;
>
> halt_rtu:
> @@ -220,6 +209,80 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
> return ret;
> }
>
> +static int prueth_emac_common_start(struct prueth *prueth)
> +{
> + struct prueth_emac *emac;
> + int ret = 0;
> + int slice;
> +
> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
> + return -EINVAL;
> +
> + /* clear SMEM and MSMC settings for all slices */
> + memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
> + memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
> +
> + icssg_class_default(prueth->miig_rt, ICSS_SLICE0, 0, false);
> + icssg_class_default(prueth->miig_rt, ICSS_SLICE1, 0, false);
> +
> + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
> + icssg_init_fw_offload_mode(prueth);
> + else
> + icssg_init_emac_mode(prueth);
> +
> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
> + emac = prueth->emac[slice];
> + if (emac) {
> + ret |= icssg_config(prueth, emac, slice);
> + if (ret)
> + return ret;
> + }
> + ret |= prueth_emac_start(prueth, slice);
> + }
> + if (!ret)
> + prueth->prus_running = 1;
> + else
> + return ret;
> +
> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
> + prueth->emac[ICSS_SLICE1];
> + ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
> + emac, IEP_DEFAULT_CYCLE_TIME_NS);
> + if (ret) {
> + dev_err(prueth->dev, "Failed to initialize IEP module\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int prueth_emac_common_stop(struct prueth *prueth)
> +{
> + struct prueth_emac *emac;
> + int slice;
> +
> + if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
> + return -EINVAL;
> +
> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE0);
> + icssg_class_disable(prueth->miig_rt, ICSS_SLICE1);
> +
> + for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
> + if (prueth->prus_running) {
> + rproc_shutdown(prueth->txpru[slice]);
> + rproc_shutdown(prueth->rtu[slice]);
> + rproc_shutdown(prueth->pru[slice]);
> + }
> + }
> + prueth->prus_running = 0;
> +
> + emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
> + prueth->emac[ICSS_SLICE1];
> + icss_iep_exit(emac->iep);
> +
> + return 0;
> +}
> +
> /* called back by PHY layer if there is change in link state of hw port*/
> static void emac_adjust_link(struct net_device *ndev)
> {
> @@ -369,12 +432,13 @@ static void prueth_iep_settime(void *clockops_data, u64 ns)
> {
> struct icssg_setclock_desc __iomem *sc_descp;
> struct prueth_emac *emac = clockops_data;
> + struct prueth *prueth = emac->prueth;
> struct icssg_setclock_desc sc_desc;
> u64 cyclecount;
> u32 cycletime;
> int timeout;
>
> - if (!emac->fw_running)
> + if (!prueth->prus_running)
> return;
>
> sc_descp = emac->prueth->shram.va + TIMESYNC_FW_WC_SETCLOCK_DESC_OFFSET;
> @@ -543,23 +607,17 @@ static int emac_ndo_open(struct net_device *ndev)
> {
> struct prueth_emac *emac = netdev_priv(ndev);
> int ret, i, num_data_chn = emac->tx_ch_num;
> + struct icssg_flow_cfg __iomem *flow_cfg;
> struct prueth *prueth = emac->prueth;
> int slice = prueth_emac_slice(emac);
> struct device *dev = prueth->dev;
> int max_rx_flows;
> int rx_flow;
>
> - /* clear SMEM and MSMC settings for all slices */
> - if (!prueth->emacs_initialized) {
> - memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
> - memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
> - }
> -
> /* set h/w MAC as user might have re-configured */
> ether_addr_copy(emac->mac_addr, ndev->dev_addr);
>
> icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
> - icssg_class_default(prueth->miig_rt, slice, 0, false);
> icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>
> /* Notify the stack of the actual queue counts. */
> @@ -597,18 +655,23 @@ static int emac_ndo_open(struct net_device *ndev)
> goto cleanup_napi;
> }
>
> - /* reset and start PRU firmware */
> - ret = prueth_emac_start(prueth, emac);
> - if (ret)
> - goto free_rx_irq;
> + if (!prueth->emacs_initialized) {
> + ret = prueth_emac_common_start(prueth);
> + if (ret)
> + goto stop;
> + }
>
> - icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
> + flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
> + writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);
> + ret = emac_fdb_flow_id_updated(emac);
>
> - if (!prueth->emacs_initialized) {
> - ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
> - emac, IEP_DEFAULT_CYCLE_TIME_NS);
> + if (ret) {
> + netdev_err(ndev, "Failed to update Rx Flow ID %d", ret);
> + goto stop;
> }
>
> + icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
> +
> ret = request_threaded_irq(emac->tx_ts_irq, NULL, prueth_tx_ts_irq,
> IRQF_ONESHOT, dev_name(dev), emac);
> if (ret)
> @@ -653,8 +716,7 @@ static int emac_ndo_open(struct net_device *ndev)
> free_tx_ts_irq:
> free_irq(emac->tx_ts_irq, emac);
> stop:
> - prueth_emac_stop(emac);
> -free_rx_irq:
> + prueth_emac_common_stop(prueth);
> free_irq(emac->rx_chns.irq[rx_flow], emac);
> cleanup_napi:
> prueth_ndev_del_tx_napi(emac, emac->tx_ch_num);
> @@ -689,8 +751,6 @@ static int emac_ndo_stop(struct net_device *ndev)
> if (ndev->phydev)
> phy_stop(ndev->phydev);
>
> - icssg_class_disable(prueth->miig_rt, prueth_emac_slice(emac));
> -
> if (emac->prueth->is_hsr_offload_mode)
> __dev_mc_unsync(ndev, icssg_prueth_hsr_del_mcast);
> else
> @@ -728,11 +788,9 @@ static int emac_ndo_stop(struct net_device *ndev)
> /* Destroying the queued work in ndo_stop() */
> cancel_delayed_work_sync(&emac->stats_work);
>
> - if (prueth->emacs_initialized == 1)
> - icss_iep_exit(emac->iep);
> -
> /* stop PRUs */
> - prueth_emac_stop(emac);
> + if (prueth->emacs_initialized == 1)
> + prueth_emac_common_stop(prueth);
>
> free_irq(emac->tx_ts_irq, emac);
>
> @@ -1069,16 +1127,10 @@ static void prueth_emac_restart(struct prueth *prueth)
> icssg_set_port_state(emac1, ICSSG_EMAC_PORT_DISABLE);
>
> /* Stop both pru cores for both PRUeth ports*/
> - prueth_emac_stop(emac0);
> - prueth->emacs_initialized--;
> - prueth_emac_stop(emac1);
> - prueth->emacs_initialized--;
> + prueth_emac_common_stop(prueth);
>
> /* Start both pru cores for both PRUeth ports */
> - prueth_emac_start(prueth, emac0);
> - prueth->emacs_initialized++;
> - prueth_emac_start(prueth, emac1);
> - prueth->emacs_initialized++;
> + prueth_emac_common_start(prueth);
>
> /* Enable forwarding for both PRUeth ports */
> icssg_set_port_state(emac0, ICSSG_EMAC_PORT_FORWARD);
> @@ -1413,13 +1465,10 @@ static int prueth_probe(struct platform_device *pdev)
> prueth->pa_stats = NULL;
> }
>
> - if (eth0_node) {
> + if (eth0_node || eth1_node) {
> ret = prueth_get_cores(prueth, ICSS_SLICE0, false);
> if (ret)
> goto put_cores;
> - }
> -
> - if (eth1_node) {
> ret = prueth_get_cores(prueth, ICSS_SLICE1, false);
> if (ret)
> goto put_cores;
> @@ -1618,14 +1667,12 @@ static int prueth_probe(struct platform_device *pdev)
> pruss_put(prueth->pruss);
>
> put_cores:
> - if (eth1_node) {
> - prueth_put_cores(prueth, ICSS_SLICE1);
> - of_node_put(eth1_node);
> - }
> -
> - if (eth0_node) {
> + if (eth0_node || eth1_node) {
> prueth_put_cores(prueth, ICSS_SLICE0);
> of_node_put(eth0_node);
> +
> + prueth_put_cores(prueth, ICSS_SLICE1);
> + of_node_put(eth1_node);
> }
>
> return ret;
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> index f5c1d473e9f9..b30f2e9a73d8 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> @@ -257,6 +257,7 @@ struct icssg_firmwares {
> * @is_switchmode_supported: indicates platform support for switch mode
> * @switch_id: ID for mapping switch ports to bridge
> * @default_vlan: Default VLAN for host
> + * @prus_running: flag to indicate if all pru cores are running
> */
> struct prueth {
> struct device *dev;
> @@ -298,6 +299,7 @@ struct prueth {
> int default_vlan;
> /** @vtbl_lock: Lock for vtbl in shared memory */
> spinlock_t vtbl_lock;
> + bool prus_running;
> };
>
> struct emac_tx_ts_response {
> @@ -361,6 +363,8 @@ int icssg_set_port_state(struct prueth_emac *emac,
> enum icssg_port_state_cmd state);
> void icssg_config_set_speed(struct prueth_emac *emac);
> void icssg_config_half_duplex(struct prueth_emac *emac);
> +void icssg_init_emac_mode(struct prueth *prueth);
> +void icssg_init_fw_offload_mode(struct prueth *prueth);
>
> /* Buffer queue helpers */
> int icssg_queue_pop(struct prueth *prueth, u8 queue);
> @@ -377,6 +381,7 @@ void icssg_vtbl_modify(struct prueth_emac *emac, u8 vid, u8 port_mask,
> u8 untag_mask, bool add);
> u16 icssg_get_pvid(struct prueth_emac *emac);
> void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port);
> +int emac_fdb_flow_id_updated(struct prueth_emac *emac);
> #define prueth_napi_to_tx_chn(pnapi) \
> container_of(pnapi, struct prueth_tx_chn, napi_tx)
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence.
@ 2024-12-05 8:24 Meghana Malladi
2024-12-05 8:30 ` Meghana Malladi
0 siblings, 1 reply; 12+ messages in thread
From: Meghana Malladi @ 2024-12-05 8:24 UTC (permalink / raw)
To: vigneshr, m-malladi, Roger Quadros, javier.carrasco.cruz,
diogo.ivo, horms, pabeni, kuba, edumazet, davem, andrew+netdev
Cc: linux-kernel, netdev, linux-arm-kernel, srk, danishanwar
From: MD Danish Anwar <danishanwar@ti.com>
Timesync related operations are ran in PRU0 cores for both ICSSG SLICE0
and SLICE1. Currently whenever any ICSSG interface comes up we load the
respective firmwares to PRU cores and whenever interface goes down, we
stop the resective cores. Due to this, when SLICE0 goes down while
SLICE1 is still active, PRU0 firmwares are unloaded and PRU0 core is
stopped. This results in clock jump for SLICE1 interface as the timesync
related operations are no longer running.
As there are interdependencies between SLICE0 and SLICE1 firmwares,
fix this by running both PRU0 and PRU1 firmwares as long as at least 1
ICSSG interface is up. Add new flag in prueth struct to check if all
firmwares are running.
Use emacs_initialized as reference count to load the firmwares for the
first and last interface up/down. Moving init_emac_mode and fw_offload_mode
API outside of icssg_config to icssg_common_start API as they need
to be called only once per firmware boot.
Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---
Hi all,
This patch is based on net-next tagged next-20241128.
v2:https://lore.kernel.org/all/20241128122931.2494446-2-m-malladi@ti.com/
* Changes since v2 (v3-v2):
- error handling in caller function of prueth_emac_common_start()
- Use prus_running flag check before stopping the firmwares
Both suggested by Roger Quadros <rogerq@kernel.org>
drivers/net/ethernet/ti/icssg/icssg_config.c | 45 ++++--
drivers/net/ethernet/ti/icssg/icssg_config.h | 1 +
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 157 ++++++++++++-------
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 5 +
4 files changed, 140 insertions(+), 68 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
index 5d2491c2943a..342150756cf7 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
@@ -397,7 +397,7 @@ static int prueth_emac_buffer_setup(struct prueth_emac *emac)
return 0;
}
-static void icssg_init_emac_mode(struct prueth *prueth)
+void icssg_init_emac_mode(struct prueth *prueth)
{
/* When the device is configured as a bridge and it is being brought
* back to the emac mode, the host mac address has to be set as 0.
@@ -406,9 +406,6 @@ static void icssg_init_emac_mode(struct prueth *prueth)
int i;
u8 mac[ETH_ALEN] = { 0 };
- 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);
@@ -423,15 +420,13 @@ static void icssg_init_emac_mode(struct prueth *prueth)
/* Clear host MAC address */
icssg_class_set_host_mac_addr(prueth->miig_rt, mac);
}
+EXPORT_SYMBOL_GPL(icssg_init_emac_mode);
-static void icssg_init_fw_offload_mode(struct prueth *prueth)
+void icssg_init_fw_offload_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);
@@ -448,6 +443,7 @@ static void icssg_init_fw_offload_mode(struct prueth *prueth)
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);
}
+EXPORT_SYMBOL_GPL(icssg_init_fw_offload_mode);
int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
{
@@ -455,11 +451,6 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
struct icssg_flow_cfg __iomem *flow_cfg;
int ret;
- if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
- icssg_init_fw_offload_mode(prueth);
- else
- icssg_init_emac_mode(prueth);
-
memset_io(config, 0, TAS_GATE_MASK_LIST0);
icssg_miig_queues_init(prueth, slice);
@@ -786,3 +777,31 @@ void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port)
writel(pvid, prueth->shram.va + EMAC_ICSSG_SWITCH_PORT0_DEFAULT_VLAN_OFFSET);
}
EXPORT_SYMBOL_GPL(icssg_set_pvid);
+
+int emac_fdb_flow_id_updated(struct prueth_emac *emac)
+{
+ struct mgmt_cmd_rsp fdb_cmd_rsp = { 0 };
+ int slice = prueth_emac_slice(emac);
+ struct mgmt_cmd fdb_cmd = { 0 };
+ int ret = 0;
+
+ fdb_cmd.header = ICSSG_FW_MGMT_CMD_HEADER;
+ fdb_cmd.type = ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW;
+ fdb_cmd.seqnum = ++(emac->prueth->icssg_hwcmdseq);
+ fdb_cmd.param = 0;
+
+ fdb_cmd.param |= (slice << 4);
+ fdb_cmd.cmd_args[0] = 0;
+
+ ret = icssg_send_fdb_msg(emac, &fdb_cmd, &fdb_cmd_rsp);
+
+ if (ret)
+ return ret;
+
+ WARN_ON(fdb_cmd.seqnum != fdb_cmd_rsp.seqnum);
+ if (fdb_cmd_rsp.status == 1)
+ return 0;
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(emac_fdb_flow_id_updated);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
index 92c2deaa3068..c884e9fa099e 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
@@ -55,6 +55,7 @@ struct icssg_rxq_ctx {
#define ICSSG_FW_MGMT_FDB_CMD_TYPE 0x03
#define ICSSG_FW_MGMT_CMD_TYPE 0x04
#define ICSSG_FW_MGMT_PKT 0x80000000
+#define ICSSG_FW_MGMT_FDB_CMD_TYPE_RX_FLOW 0x05
struct icssg_r30_cmd {
u32 cmd[4];
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index c568c84a032b..2e22e793b01a 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -164,11 +164,11 @@ static struct icssg_firmwares icssg_emac_firmwares[] = {
}
};
-static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
+static int prueth_emac_start(struct prueth *prueth, int slice)
{
struct icssg_firmwares *firmwares;
struct device *dev = prueth->dev;
- int slice, ret;
+ int ret;
if (prueth->is_switch_mode)
firmwares = icssg_switch_firmwares;
@@ -177,16 +177,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
else
firmwares = icssg_emac_firmwares;
- slice = prueth_emac_slice(emac);
- if (slice < 0) {
- netdev_err(emac->ndev, "invalid port\n");
- return -EINVAL;
- }
-
- ret = icssg_config(prueth, emac, slice);
- if (ret)
- return ret;
-
ret = rproc_set_firmware(prueth->pru[slice], firmwares[slice].pru);
ret = rproc_boot(prueth->pru[slice]);
if (ret) {
@@ -208,7 +198,6 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
goto halt_rtu;
}
- emac->fw_running = 1;
return 0;
halt_rtu:
@@ -220,6 +209,80 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
return ret;
}
+static int prueth_emac_common_start(struct prueth *prueth)
+{
+ struct prueth_emac *emac;
+ int ret = 0;
+ int slice;
+
+ if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
+ return -EINVAL;
+
+ /* clear SMEM and MSMC settings for all slices */
+ memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
+ memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
+
+ icssg_class_default(prueth->miig_rt, ICSS_SLICE0, 0, false);
+ icssg_class_default(prueth->miig_rt, ICSS_SLICE1, 0, false);
+
+ if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
+ icssg_init_fw_offload_mode(prueth);
+ else
+ icssg_init_emac_mode(prueth);
+
+ for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
+ emac = prueth->emac[slice];
+ if (emac) {
+ ret |= icssg_config(prueth, emac, slice);
+ if (ret)
+ return ret;
+ }
+ ret |= prueth_emac_start(prueth, slice);
+ }
+ if (!ret)
+ prueth->prus_running = 1;
+ else
+ return ret;
+
+ emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
+ prueth->emac[ICSS_SLICE1];
+ ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
+ emac, IEP_DEFAULT_CYCLE_TIME_NS);
+ if (ret) {
+ dev_err(prueth->dev, "Failed to initialize IEP module\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int prueth_emac_common_stop(struct prueth *prueth)
+{
+ struct prueth_emac *emac;
+ int slice;
+
+ if (!prueth->emac[ICSS_SLICE0] && !prueth->emac[ICSS_SLICE1])
+ return -EINVAL;
+
+ icssg_class_disable(prueth->miig_rt, ICSS_SLICE0);
+ icssg_class_disable(prueth->miig_rt, ICSS_SLICE1);
+
+ for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
+ if (prueth->prus_running) {
+ rproc_shutdown(prueth->txpru[slice]);
+ rproc_shutdown(prueth->rtu[slice]);
+ rproc_shutdown(prueth->pru[slice]);
+ }
+ }
+ prueth->prus_running = 0;
+
+ emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
+ prueth->emac[ICSS_SLICE1];
+ icss_iep_exit(emac->iep);
+
+ return 0;
+}
+
/* called back by PHY layer if there is change in link state of hw port*/
static void emac_adjust_link(struct net_device *ndev)
{
@@ -369,12 +432,13 @@ static void prueth_iep_settime(void *clockops_data, u64 ns)
{
struct icssg_setclock_desc __iomem *sc_descp;
struct prueth_emac *emac = clockops_data;
+ struct prueth *prueth = emac->prueth;
struct icssg_setclock_desc sc_desc;
u64 cyclecount;
u32 cycletime;
int timeout;
- if (!emac->fw_running)
+ if (!prueth->prus_running)
return;
sc_descp = emac->prueth->shram.va + TIMESYNC_FW_WC_SETCLOCK_DESC_OFFSET;
@@ -543,23 +607,17 @@ static int emac_ndo_open(struct net_device *ndev)
{
struct prueth_emac *emac = netdev_priv(ndev);
int ret, i, num_data_chn = emac->tx_ch_num;
+ struct icssg_flow_cfg __iomem *flow_cfg;
struct prueth *prueth = emac->prueth;
int slice = prueth_emac_slice(emac);
struct device *dev = prueth->dev;
int max_rx_flows;
int rx_flow;
- /* clear SMEM and MSMC settings for all slices */
- if (!prueth->emacs_initialized) {
- memset_io(prueth->msmcram.va, 0, prueth->msmcram.size);
- memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS);
- }
-
/* set h/w MAC as user might have re-configured */
ether_addr_copy(emac->mac_addr, ndev->dev_addr);
icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
- icssg_class_default(prueth->miig_rt, slice, 0, false);
icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
/* Notify the stack of the actual queue counts. */
@@ -597,18 +655,23 @@ static int emac_ndo_open(struct net_device *ndev)
goto cleanup_napi;
}
- /* reset and start PRU firmware */
- ret = prueth_emac_start(prueth, emac);
- if (ret)
- goto free_rx_irq;
+ if (!prueth->emacs_initialized) {
+ ret = prueth_emac_common_start(prueth);
+ if (ret)
+ goto stop;
+ }
- icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
+ flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
+ writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);
+ ret = emac_fdb_flow_id_updated(emac);
- if (!prueth->emacs_initialized) {
- ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
- emac, IEP_DEFAULT_CYCLE_TIME_NS);
+ if (ret) {
+ netdev_err(ndev, "Failed to update Rx Flow ID %d", ret);
+ goto stop;
}
+ icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
+
ret = request_threaded_irq(emac->tx_ts_irq, NULL, prueth_tx_ts_irq,
IRQF_ONESHOT, dev_name(dev), emac);
if (ret)
@@ -653,8 +716,7 @@ static int emac_ndo_open(struct net_device *ndev)
free_tx_ts_irq:
free_irq(emac->tx_ts_irq, emac);
stop:
- prueth_emac_stop(emac);
-free_rx_irq:
+ prueth_emac_common_stop(prueth);
free_irq(emac->rx_chns.irq[rx_flow], emac);
cleanup_napi:
prueth_ndev_del_tx_napi(emac, emac->tx_ch_num);
@@ -689,8 +751,6 @@ static int emac_ndo_stop(struct net_device *ndev)
if (ndev->phydev)
phy_stop(ndev->phydev);
- icssg_class_disable(prueth->miig_rt, prueth_emac_slice(emac));
-
if (emac->prueth->is_hsr_offload_mode)
__dev_mc_unsync(ndev, icssg_prueth_hsr_del_mcast);
else
@@ -728,11 +788,9 @@ static int emac_ndo_stop(struct net_device *ndev)
/* Destroying the queued work in ndo_stop() */
cancel_delayed_work_sync(&emac->stats_work);
- if (prueth->emacs_initialized == 1)
- icss_iep_exit(emac->iep);
-
/* stop PRUs */
- prueth_emac_stop(emac);
+ if (prueth->emacs_initialized == 1)
+ prueth_emac_common_stop(prueth);
free_irq(emac->tx_ts_irq, emac);
@@ -1069,16 +1127,10 @@ static void prueth_emac_restart(struct prueth *prueth)
icssg_set_port_state(emac1, ICSSG_EMAC_PORT_DISABLE);
/* Stop both pru cores for both PRUeth ports*/
- prueth_emac_stop(emac0);
- prueth->emacs_initialized--;
- prueth_emac_stop(emac1);
- prueth->emacs_initialized--;
+ prueth_emac_common_stop(prueth);
/* Start both pru cores for both PRUeth ports */
- prueth_emac_start(prueth, emac0);
- prueth->emacs_initialized++;
- prueth_emac_start(prueth, emac1);
- prueth->emacs_initialized++;
+ prueth_emac_common_start(prueth);
/* Enable forwarding for both PRUeth ports */
icssg_set_port_state(emac0, ICSSG_EMAC_PORT_FORWARD);
@@ -1413,13 +1465,10 @@ static int prueth_probe(struct platform_device *pdev)
prueth->pa_stats = NULL;
}
- if (eth0_node) {
+ if (eth0_node || eth1_node) {
ret = prueth_get_cores(prueth, ICSS_SLICE0, false);
if (ret)
goto put_cores;
- }
-
- if (eth1_node) {
ret = prueth_get_cores(prueth, ICSS_SLICE1, false);
if (ret)
goto put_cores;
@@ -1618,14 +1667,12 @@ static int prueth_probe(struct platform_device *pdev)
pruss_put(prueth->pruss);
put_cores:
- if (eth1_node) {
- prueth_put_cores(prueth, ICSS_SLICE1);
- of_node_put(eth1_node);
- }
-
- if (eth0_node) {
+ if (eth0_node || eth1_node) {
prueth_put_cores(prueth, ICSS_SLICE0);
of_node_put(eth0_node);
+
+ prueth_put_cores(prueth, ICSS_SLICE1);
+ of_node_put(eth1_node);
}
return ret;
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index f5c1d473e9f9..b30f2e9a73d8 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -257,6 +257,7 @@ struct icssg_firmwares {
* @is_switchmode_supported: indicates platform support for switch mode
* @switch_id: ID for mapping switch ports to bridge
* @default_vlan: Default VLAN for host
+ * @prus_running: flag to indicate if all pru cores are running
*/
struct prueth {
struct device *dev;
@@ -298,6 +299,7 @@ struct prueth {
int default_vlan;
/** @vtbl_lock: Lock for vtbl in shared memory */
spinlock_t vtbl_lock;
+ bool prus_running;
};
struct emac_tx_ts_response {
@@ -361,6 +363,8 @@ int icssg_set_port_state(struct prueth_emac *emac,
enum icssg_port_state_cmd state);
void icssg_config_set_speed(struct prueth_emac *emac);
void icssg_config_half_duplex(struct prueth_emac *emac);
+void icssg_init_emac_mode(struct prueth *prueth);
+void icssg_init_fw_offload_mode(struct prueth *prueth);
/* Buffer queue helpers */
int icssg_queue_pop(struct prueth *prueth, u8 queue);
@@ -377,6 +381,7 @@ void icssg_vtbl_modify(struct prueth_emac *emac, u8 vid, u8 port_mask,
u8 untag_mask, bool add);
u16 icssg_get_pvid(struct prueth_emac *emac);
void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port);
+int emac_fdb_flow_id_updated(struct prueth_emac *emac);
#define prueth_napi_to_tx_chn(pnapi) \
container_of(pnapi, struct prueth_tx_chn, napi_tx)
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-12-09 14:45 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-12-05 8:28 [PATCH net v3 0/2] IEP clock module bug fixes Meghana Malladi
2024-12-05 8:28 ` [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence Meghana Malladi
2024-12-05 9:10 ` Kalesh Anakkur Purayil
2024-12-09 10:39 ` [EXTERNAL] " Meghana Malladi
2024-12-05 13:08 ` Roger Quadros
2024-12-09 10:34 ` Meghana Malladi
2024-12-09 12:39 ` Roger Quadros
2024-12-09 13:01 ` Meghana Malladi
2024-12-09 14:44 ` Diogo Ivo
2024-12-05 8:28 ` [PATCH net v3 2/2] net: ti: icssg-prueth: Fix clearing of IEP_CMP_CFG registers during iep_init Meghana Malladi
-- strict thread matches above, loose matches on Subject: below --
2024-12-05 8:24 [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence Meghana Malladi
2024-12-05 8:30 ` Meghana Malladi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®