* [PATCH net-next v5 0/5] Introduce HSR offload support for ICSSG
@ 2024-09-06 11:15 MD Danish Anwar
2024-09-06 11:15 ` [PATCH net-next v5 1/5] net: ti: icss-iep: Move icss_iep structure MD Danish Anwar
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: MD Danish Anwar @ 2024-09-06 11:15 UTC (permalink / raw)
To: robh, jan.kiszka, dan.carpenter, saikrishnag, andrew,
javier.carrasco.cruz, jacob.e.keller, diogo.ivo, horms,
richardcochran, pabeni, kuba, edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
Roger Quadros, danishanwar
Hi All,
This series introduces HSR offload support for ICSSG driver. To support HSR
offload to hardware, ICSSG HSR firmware is used.
This series introduces,
1. HSR frame offload support for ICSSG driver.
2. HSR Tx Packet duplication offload
3. HSR Tx Tag and Rx Tag offload
4. Multicast filtering support in HSR offload mode.
5. Dependencies related to IEP.
HSR Test Setup:
--------------
___________ ___________ ___________
| | Link AB | | Link BC | |
__| AM64* |_________| AM64 |_________| AM64* |___
| | Station A | | Station B | | Station C | |
| |___________| |___________| |___________| |
| |
|______________________________________________________________|
Link CA
*Could be any device that supports two ethernet interfaces.
Steps to switch to HSR frame forward offload mode:
-------------------------------------------------
Example assuming eth1, eth2 ports of ICSSG1 on AM64-EVM
1) Enable HSR offload for both interfaces
ethtool -K eth1 hsr-fwd-offload on
ethtool -K eth1 hsr-dup-offload on
ethtool -K eth1 hsr-tag-ins-offload on
ethtool -K eth1 hsr-tag-rm-offload on
ethtool -K eth2 hsr-fwd-offload on
ethtool -K eth2 hsr-dup-offload on
ethtool -K eth2 hsr-tag-ins-offload on
ethtool -K eth2 hsr-tag-rm-offload on
2) Create HSR interface and add slave interfaces to it
ip link add name hsr0 type hsr slave1 eth1 slave2 eth2 \
supervision 45 version 1
3) Add IP address to the HSR interface
ip addr add <IP_ADDR>/24 dev hsr0
4) Bring up the HSR interface
ip link set hsr0 up
Switching back to previous mode:
--------------------------------
1) Delete HSR interface
ip link delete hsr0
2) Disable HSR port-to-port offloading mode, packet duplication
ethtool -K eth1 hsr-fwd-offload off
ethtool -K eth1 hsr-dup-offload off
ethtool -K eth1 hsr-tag-ins-offload off
ethtool -K eth1 hsr-tag-rm-offload off
ethtool -K eth2 hsr-fwd-offload off
ethtool -K eth2 hsr-dup-offload off
ethtool -K eth2 hsr-tag-ins-offload off
ethtool -K eth2 hsr-tag-rm-offload off
Testing the port-to-port frame forward offload feature:
-----------------------------------------------------
1) Connect the LAN cables as shown in the test setup.
2) Configure Station A and Station C in HSR non-offload mode.
3) Configure Station B is HSR offload mode.
4) Since HSR is a redundancy protocol, disconnect cable "Link CA",
to ensure frames from Station A reach Station C only through
Station B.
5) Run iperf3 Server on Station C and client on station A.
7) Check the CPU usage on Station B.
CPU usage report on Station B using mpstat when running UDP iperf3:
-------------------------------------------------------------------
1) Non-Offload case
-------------------
CPU %usr %nice %sys %iowait %irq %soft %steal %guest %idle
all 0.00 0.00 0.50 0.00 3.52 29.15 0.00 0.00 66.83
0 0.00 0.00 0.00 0.00 7.00 58.00 0.00 0.00 35.00
1 0.00 0.00 0.99 0.00 0.99 0.00 0.00 0.00 98.02
2) Offload case
---------------
CPU %usr %nice %sys %iowait %irq %soft %steal %guest %idle
all 0.00 0.00 0.00 0.00 0.50 0.00 0.00 0.00 99.50
0 0.00 0.00 0.99 0.00 0.00 0.00 0.00 0.00 99.01
1 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 100.00
Note:
1) At the very least, hsr-fwd-offload must be enabled.
Without offloading the port-to-port offload, other
HSR offloads cannot be enabled.
2) Inorder to enable hsr-tag-ins-offload, hsr-dup-offload
must also be enabled as these are tightly coupled in
the firmware implementation.
Changes from v4 to v5:
*) Fixed warning reported by kernel test robot [0].
Changes from v3 to v4:
*) Simplified ndo_set_features() implementation as suggested by Roger Quadros
<rogerq@kernel.org>
*) Maintained 80 character limit per line wherever possible.
*) Returning -EOPNOTSUPP whenever offloading is not possible.
*) Changed dev_dbg() prints to netdev_dbg() as suggested by Alexander
Lobakin <aleksander.lobakin@intel.com>
*) Squashed patch [1] and [2] together as hsr tag ins is dependent on hsr
tx duplication. Also didn't add Roger's RB tag from patch [2] as the patch
is now merged with [1] in patch 4/5.
*) Implemented emac_ndo_fix_features() to make sure NETIF_F_HW_HSR_DUP is
enabled / disabled with NETIF_F_HW_HSR_TAG_INS as suggested by Roger
Quadros <rogerq@kernel.org>
*) Added Roger's RB tag to patch 5/5.
*) Modified commit message of patch 3/5 to state that driver will be back
to previously used mode once HSR is disabled.
Changes from v2 to v3:
*) Renamed APIs common to switch and hsr modes with suffix _fw_offload.
*) Returning EOPNOTSUPP in prueth_hsr_port_link() as suggested by
Andrew Lunn <andrew@lunn.ch>
*) Dropped unneccassary dev_err prints and changed dev_err to dev_dbg
where applicable.
*) Renamed NETIF_PRUETH_HSR_OFFLOAD to NETIF_PRUETH_HSR_OFFLOAD_FEATURES
to make it clear it is a collection of features, not an alias for one
feature.
*) Added Kernel doc entries for @hsr_members and @is_hsr_offload_mode as
suggested by Simon Horman <horms@kernel.org>
*) Dropped patch [3] as it is no longer needed in this series. It is
already merged to net/main by commit [4].
*) Collected Reviewed-by tag from Roger Quadros <rogerq@kernel.org> for
PATCH 1/6 and PATCH 2/6.
*) Added if check for current mode before calling __dev_mc_unsync as
suggested by Roger Quadros <rogerq@kernel.org>
*) Updated commit message of PATCH 6/6 to describe handling of duplicate
discard in the driver.
Changes from v1 to v2:
*) Modified patch 2/7 to only contain code movement as suggested by
Dan Carpenter <dan.carpenter@linaro.org>
*) Added patch 3/7 by splitting it from 2/6 as the patch is not part of
code movement done in patch 2/7.
*) Rebased on latest net-next/main.
v1: https://lore.kernel.org/all/20240808110800.1281716-1-danishanwar@ti.com/
v2: https://lore.kernel.org/all/20240813074233.2473876-1-danishanwar@ti.com
v3: https://lore.kernel.org/all/20240828091901.3120935-1-danishanwar@ti.com/
v4: https://lore.kernel.org/all/20240904100506.3665892-1-danishanwar@ti.com/
[0] https://lore.kernel.org/all/202409061658.vSwcFJiK-lkp@intel.com/
[1] https://lore.kernel.org/all/20240828091901.3120935-5-danishanwar@ti.com/
[2] https://lore.kernel.org/all/20240828091901.3120935-7-danishanwar@ti.com/
[3] https://lore.kernel.org/all/20240813074233.2473876-2-danishanwar@ti.com/
[4] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=e846be0fba85
MD Danish Anwar (4):
net: ti: icss-iep: Move icss_iep structure
net: ti: icssg-prueth: Stop hardcoding def_inc
net: ti: icssg-prueth: Add support for HSR frame forward offload
net: ti: icssg-prueth: Add multicast filtering support in HSR mode
Ravi Gunasekaran (1):
net: ti: icssg-prueth: Enable HSR Tx duplication, Tx Tag and Rx Tag
offload
drivers/net/ethernet/ti/icssg/icss_iep.c | 72 -------
drivers/net/ethernet/ti/icssg/icss_iep.h | 73 ++++++-
.../net/ethernet/ti/icssg/icssg_classifier.c | 1 +
drivers/net/ethernet/ti/icssg/icssg_common.c | 18 +-
drivers/net/ethernet/ti/icssg/icssg_config.c | 22 +-
drivers/net/ethernet/ti/icssg/icssg_config.h | 2 +
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 203 +++++++++++++++++-
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 9 +
8 files changed, 308 insertions(+), 92 deletions(-)
base-commit: 52fc70a32573707f70d6b1b5c5fe85cc91457393
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v5 1/5] net: ti: icss-iep: Move icss_iep structure
2024-09-06 11:15 [PATCH net-next v5 0/5] Introduce HSR offload support for ICSSG MD Danish Anwar
@ 2024-09-06 11:15 ` MD Danish Anwar
2024-09-06 11:15 ` [PATCH net-next v5 2/5] net: ti: icssg-prueth: Stop hardcoding def_inc MD Danish Anwar
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: MD Danish Anwar @ 2024-09-06 11:15 UTC (permalink / raw)
To: robh, jan.kiszka, dan.carpenter, saikrishnag, andrew,
javier.carrasco.cruz, jacob.e.keller, diogo.ivo, horms,
richardcochran, pabeni, kuba, edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
Roger Quadros, danishanwar
Move icss_iep structure definition and to icss_iep.h file so that the
structure members can be used / accessed by all icssg driver files.
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Roger Quadros <rogerq@kernel.org>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
drivers/net/ethernet/ti/icssg/icss_iep.c | 72 -----------------------
drivers/net/ethernet/ti/icssg/icss_iep.h | 73 +++++++++++++++++++++++-
2 files changed, 72 insertions(+), 73 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.c b/drivers/net/ethernet/ti/icssg/icss_iep.c
index 75c294ce6fb6..5d6d1cf78e93 100644
--- a/drivers/net/ethernet/ti/icssg/icss_iep.c
+++ b/drivers/net/ethernet/ti/icssg/icss_iep.c
@@ -53,78 +53,6 @@
#define IEP_CAP_CFG_CAPNR_1ST_EVENT_EN(n) BIT(LATCH_INDEX(n))
#define IEP_CAP_CFG_CAP_ASYNC_EN(n) BIT(LATCH_INDEX(n) + 10)
-enum {
- ICSS_IEP_GLOBAL_CFG_REG,
- ICSS_IEP_GLOBAL_STATUS_REG,
- ICSS_IEP_COMPEN_REG,
- ICSS_IEP_SLOW_COMPEN_REG,
- ICSS_IEP_COUNT_REG0,
- ICSS_IEP_COUNT_REG1,
- ICSS_IEP_CAPTURE_CFG_REG,
- ICSS_IEP_CAPTURE_STAT_REG,
-
- ICSS_IEP_CAP6_RISE_REG0,
- ICSS_IEP_CAP6_RISE_REG1,
-
- ICSS_IEP_CAP7_RISE_REG0,
- ICSS_IEP_CAP7_RISE_REG1,
-
- ICSS_IEP_CMP_CFG_REG,
- ICSS_IEP_CMP_STAT_REG,
- ICSS_IEP_CMP0_REG0,
- ICSS_IEP_CMP0_REG1,
- ICSS_IEP_CMP1_REG0,
- ICSS_IEP_CMP1_REG1,
-
- ICSS_IEP_CMP8_REG0,
- ICSS_IEP_CMP8_REG1,
- ICSS_IEP_SYNC_CTRL_REG,
- ICSS_IEP_SYNC0_STAT_REG,
- ICSS_IEP_SYNC1_STAT_REG,
- ICSS_IEP_SYNC_PWIDTH_REG,
- ICSS_IEP_SYNC0_PERIOD_REG,
- ICSS_IEP_SYNC1_DELAY_REG,
- ICSS_IEP_SYNC_START_REG,
- ICSS_IEP_MAX_REGS,
-};
-
-/**
- * struct icss_iep_plat_data - Plat data to handle SoC variants
- * @config: Regmap configuration data
- * @reg_offs: register offsets to capture offset differences across SoCs
- * @flags: Flags to represent IEP properties
- */
-struct icss_iep_plat_data {
- const struct regmap_config *config;
- u32 reg_offs[ICSS_IEP_MAX_REGS];
- u32 flags;
-};
-
-struct icss_iep {
- struct device *dev;
- void __iomem *base;
- const struct icss_iep_plat_data *plat_data;
- struct regmap *map;
- struct device_node *client_np;
- unsigned long refclk_freq;
- int clk_tick_time; /* one refclk tick time in ns */
- struct ptp_clock_info ptp_info;
- struct ptp_clock *ptp_clock;
- struct mutex ptp_clk_mutex; /* PHC access serializer */
- u32 def_inc;
- s16 slow_cmp_inc;
- u32 slow_cmp_count;
- const struct icss_iep_clockops *ops;
- void *clockops_data;
- u32 cycle_time_ns;
- u32 perout_enabled;
- bool pps_enabled;
- int cap_cmp_irq;
- u64 period;
- u32 latch_enable;
- struct work_struct work;
-};
-
/**
* icss_iep_get_count_hi() - Get the upper 32 bit IEP counter
* @iep: Pointer to structure representing IEP.
diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.h b/drivers/net/ethernet/ti/icssg/icss_iep.h
index 803a4b714893..0bdca0155abd 100644
--- a/drivers/net/ethernet/ti/icssg/icss_iep.h
+++ b/drivers/net/ethernet/ti/icssg/icss_iep.h
@@ -12,7 +12,78 @@
#include <linux/ptp_clock_kernel.h>
#include <linux/regmap.h>
-struct icss_iep;
+enum {
+ ICSS_IEP_GLOBAL_CFG_REG,
+ ICSS_IEP_GLOBAL_STATUS_REG,
+ ICSS_IEP_COMPEN_REG,
+ ICSS_IEP_SLOW_COMPEN_REG,
+ ICSS_IEP_COUNT_REG0,
+ ICSS_IEP_COUNT_REG1,
+ ICSS_IEP_CAPTURE_CFG_REG,
+ ICSS_IEP_CAPTURE_STAT_REG,
+
+ ICSS_IEP_CAP6_RISE_REG0,
+ ICSS_IEP_CAP6_RISE_REG1,
+
+ ICSS_IEP_CAP7_RISE_REG0,
+ ICSS_IEP_CAP7_RISE_REG1,
+
+ ICSS_IEP_CMP_CFG_REG,
+ ICSS_IEP_CMP_STAT_REG,
+ ICSS_IEP_CMP0_REG0,
+ ICSS_IEP_CMP0_REG1,
+ ICSS_IEP_CMP1_REG0,
+ ICSS_IEP_CMP1_REG1,
+
+ ICSS_IEP_CMP8_REG0,
+ ICSS_IEP_CMP8_REG1,
+ ICSS_IEP_SYNC_CTRL_REG,
+ ICSS_IEP_SYNC0_STAT_REG,
+ ICSS_IEP_SYNC1_STAT_REG,
+ ICSS_IEP_SYNC_PWIDTH_REG,
+ ICSS_IEP_SYNC0_PERIOD_REG,
+ ICSS_IEP_SYNC1_DELAY_REG,
+ ICSS_IEP_SYNC_START_REG,
+ ICSS_IEP_MAX_REGS,
+};
+
+/**
+ * struct icss_iep_plat_data - Plat data to handle SoC variants
+ * @config: Regmap configuration data
+ * @reg_offs: register offsets to capture offset differences across SoCs
+ * @flags: Flags to represent IEP properties
+ */
+struct icss_iep_plat_data {
+ const struct regmap_config *config;
+ u32 reg_offs[ICSS_IEP_MAX_REGS];
+ u32 flags;
+};
+
+struct icss_iep {
+ struct device *dev;
+ void __iomem *base;
+ const struct icss_iep_plat_data *plat_data;
+ struct regmap *map;
+ struct device_node *client_np;
+ unsigned long refclk_freq;
+ int clk_tick_time; /* one refclk tick time in ns */
+ struct ptp_clock_info ptp_info;
+ struct ptp_clock *ptp_clock;
+ struct mutex ptp_clk_mutex; /* PHC access serializer */
+ u32 def_inc;
+ s16 slow_cmp_inc;
+ u32 slow_cmp_count;
+ const struct icss_iep_clockops *ops;
+ void *clockops_data;
+ u32 cycle_time_ns;
+ u32 perout_enabled;
+ bool pps_enabled;
+ int cap_cmp_irq;
+ u64 period;
+ u32 latch_enable;
+ struct work_struct work;
+};
+
extern const struct icss_iep_clockops prueth_iep_clockops;
/* Firmware specific clock operations */
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next v5 2/5] net: ti: icssg-prueth: Stop hardcoding def_inc
2024-09-06 11:15 [PATCH net-next v5 0/5] Introduce HSR offload support for ICSSG MD Danish Anwar
2024-09-06 11:15 ` [PATCH net-next v5 1/5] net: ti: icss-iep: Move icss_iep structure MD Danish Anwar
@ 2024-09-06 11:15 ` MD Danish Anwar
2024-09-06 11:15 ` [PATCH net-next v5 3/5] net: ti: icssg-prueth: Add support for HSR frame forward offload MD Danish Anwar
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: MD Danish Anwar @ 2024-09-06 11:15 UTC (permalink / raw)
To: robh, jan.kiszka, dan.carpenter, saikrishnag, andrew,
javier.carrasco.cruz, jacob.e.keller, diogo.ivo, horms,
richardcochran, pabeni, kuba, edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
Roger Quadros, danishanwar
The def_inc is stored in icss_iep structure. Currently default increment
(ns per clock tick) is hardcoded to 4 (Clock frequency being 250 MHz).
Change this to use the iep->def_inc variable as the iep structure is now
accessible to the driver files.
Reviewed-by: Roger Quadros <rogerq@kernel.org>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index becdda143c19..1dcd68eefdc3 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -365,7 +365,8 @@ static void prueth_iep_settime(void *clockops_data, u64 ns)
sc_desc.cyclecounter0_set = cyclecount & GENMASK(31, 0);
sc_desc.cyclecounter1_set = (cyclecount & GENMASK(63, 32)) >> 32;
sc_desc.iepcount_set = ns % cycletime;
- sc_desc.CMP0_current = cycletime - 4; //Count from 0 to (cycle time)-4
+ /* Count from 0 to (cycle time) - emac->iep->def_inc */
+ sc_desc.CMP0_current = cycletime - emac->iep->def_inc;
memcpy_toio(sc_descp, &sc_desc, sizeof(sc_desc));
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next v5 3/5] net: ti: icssg-prueth: Add support for HSR frame forward offload
2024-09-06 11:15 [PATCH net-next v5 0/5] Introduce HSR offload support for ICSSG MD Danish Anwar
2024-09-06 11:15 ` [PATCH net-next v5 1/5] net: ti: icss-iep: Move icss_iep structure MD Danish Anwar
2024-09-06 11:15 ` [PATCH net-next v5 2/5] net: ti: icssg-prueth: Stop hardcoding def_inc MD Danish Anwar
@ 2024-09-06 11:15 ` MD Danish Anwar
2024-09-10 12:12 ` Roger Quadros
2024-09-06 11:15 ` [PATCH net-next v5 4/5] net: ti: icssg-prueth: Enable HSR Tx duplication, Tx Tag and Rx Tag offload MD Danish Anwar
2024-09-06 11:15 ` [PATCH net-next v5 5/5] net: ti: icssg-prueth: Add multicast filtering support in HSR mode MD Danish Anwar
4 siblings, 1 reply; 13+ messages in thread
From: MD Danish Anwar @ 2024-09-06 11:15 UTC (permalink / raw)
To: robh, jan.kiszka, dan.carpenter, saikrishnag, andrew,
javier.carrasco.cruz, jacob.e.keller, diogo.ivo, horms,
richardcochran, pabeni, kuba, edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
Roger Quadros, danishanwar
Add support for offloading HSR port-to-port frame forward to hardware.
When the slave interfaces are added to the HSR interface, the PRU cores
will be stopped and ICSSG HSR firmwares will be loaded to them.
Similarly, when HSR interface is deleted, the PRU cores will be
restarted and the last used firmwares will be reloaded. PRUeth
interfaces will be back to the last used mode.
This commit also renames some APIs that are common between switch and
hsr mode with '_fw_offload' suffix.
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
.../net/ethernet/ti/icssg/icssg_classifier.c | 1 +
drivers/net/ethernet/ti/icssg/icssg_config.c | 18 +--
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 132 +++++++++++++++++-
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 6 +
4 files changed, 145 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_classifier.c b/drivers/net/ethernet/ti/icssg/icssg_classifier.c
index 9ec504d976d6..833ca86d0b71 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_classifier.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_classifier.c
@@ -290,6 +290,7 @@ void icssg_class_set_host_mac_addr(struct regmap *miig_rt, const u8 *mac)
mac[2] << 16 | mac[3] << 24));
regmap_write(miig_rt, MAC_INTERFACE_1, (u32)(mac[4] | mac[5] << 8));
}
+EXPORT_SYMBOL_GPL(icssg_class_set_host_mac_addr);
void icssg_class_set_mac_addr(struct regmap *miig_rt, int slice, u8 *mac)
{
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
index dae52a83a378..7b2e6c192ff3 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
@@ -107,7 +107,7 @@ static const struct map hwq_map[2][ICSSG_NUM_OTHER_QUEUES] = {
},
};
-static void icssg_config_mii_init_switch(struct prueth_emac *emac)
+static void icssg_config_mii_init_fw_offload(struct prueth_emac *emac)
{
struct prueth *prueth = emac->prueth;
int mii = prueth_emac_slice(emac);
@@ -278,7 +278,7 @@ static int emac_r30_is_done(struct prueth_emac *emac)
return 1;
}
-static int prueth_switch_buffer_setup(struct prueth_emac *emac)
+static int prueth_fw_offload_buffer_setup(struct prueth_emac *emac)
{
struct icssg_buffer_pool_cfg __iomem *bpool_cfg;
struct icssg_rxq_ctx __iomem *rxq_ctx;
@@ -424,7 +424,7 @@ static void icssg_init_emac_mode(struct prueth *prueth)
icssg_class_set_host_mac_addr(prueth->miig_rt, mac);
}
-static void icssg_init_switch_mode(struct prueth *prueth)
+static void icssg_init_fw_offload_mode(struct prueth *prueth)
{
u32 addr = prueth->shram.pa + EMAC_ICSSG_SWITCH_DEFAULT_VLAN_TABLE_OFFSET;
int i;
@@ -455,8 +455,8 @@ 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)
- icssg_init_switch_mode(prueth);
+ if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
+ icssg_init_fw_offload_mode(prueth);
else
icssg_init_emac_mode(prueth);
@@ -472,8 +472,8 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
regmap_update_bits(prueth->miig_rt, ICSSG_CFG_OFFSET,
ICSSG_CFG_DEFAULT, ICSSG_CFG_DEFAULT);
icssg_miig_set_interface_mode(prueth->miig_rt, slice, emac->phy_if);
- if (prueth->is_switch_mode)
- icssg_config_mii_init_switch(emac);
+ if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
+ icssg_config_mii_init_fw_offload(emac);
else
icssg_config_mii_init(emac);
icssg_config_ipg(emac);
@@ -498,8 +498,8 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice)
writeb(0, config + SPL_PKT_DEFAULT_PRIORITY);
writeb(0, config + QUEUE_NUM_UNTAGGED);
- if (prueth->is_switch_mode)
- ret = prueth_switch_buffer_setup(emac);
+ if (prueth->is_switch_mode || prueth->is_hsr_offload_mode)
+ ret = prueth_fw_offload_buffer_setup(emac);
else
ret = prueth_emac_buffer_setup(emac);
if (ret)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 1dcd68eefdc3..676168d6fded 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -13,6 +13,7 @@
#include <linux/dma/ti-cppi5.h>
#include <linux/etherdevice.h>
#include <linux/genalloc.h>
+#include <linux/if_hsr.h>
#include <linux/if_vlan.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
@@ -40,6 +41,8 @@
#define DEFAULT_PORT_MASK 1
#define DEFAULT_UNTAG_MASK 1
+#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES NETIF_F_HW_HSR_FWD
+
/* CTRLMMR_ICSSG_RGMII_CTRL register bits */
#define ICSSG_CTRL_RGMII_ID_MODE BIT(24)
@@ -118,6 +121,19 @@ static irqreturn_t prueth_tx_ts_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static struct icssg_firmwares icssg_hsr_firmwares[] = {
+ {
+ .pru = "ti-pruss/am65x-sr2-pru0-pruhsr-fw.elf",
+ .rtu = "ti-pruss/am65x-sr2-rtu0-pruhsr-fw.elf",
+ .txpru = "ti-pruss/am65x-sr2-txpru0-pruhsr-fw.elf",
+ },
+ {
+ .pru = "ti-pruss/am65x-sr2-pru1-pruhsr-fw.elf",
+ .rtu = "ti-pruss/am65x-sr2-rtu1-pruhsr-fw.elf",
+ .txpru = "ti-pruss/am65x-sr2-txpru1-pruhsr-fw.elf",
+ }
+};
+
static struct icssg_firmwares icssg_switch_firmwares[] = {
{
.pru = "ti-pruss/am65x-sr2-pru0-prusw-fw.elf",
@@ -152,6 +168,8 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac)
if (prueth->is_switch_mode)
firmwares = icssg_switch_firmwares;
+ else if (prueth->is_hsr_offload_mode)
+ firmwares = icssg_hsr_firmwares;
else
firmwares = icssg_emac_firmwares;
@@ -726,6 +744,31 @@ static void emac_ndo_set_rx_mode(struct net_device *ndev)
queue_work(emac->cmd_wq, &emac->rx_mode_work);
}
+static void emac_change_hsr_feature(struct net_device *ndev,
+ netdev_features_t features,
+ u64 hsr_feature)
+{
+ netdev_features_t changed = ndev->features ^ features;
+
+ if (changed & hsr_feature) {
+ if (features & hsr_feature)
+ ndev->features |= hsr_feature;
+ else
+ ndev->features &= ~hsr_feature;
+ }
+}
+
+static int emac_ndo_set_features(struct net_device *ndev,
+ netdev_features_t features)
+{
+ emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_FWD);
+ emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_DUP);
+ emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_TAG_INS);
+ emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_TAG_RM);
+
+ return 0;
+}
+
static const struct net_device_ops emac_netdev_ops = {
.ndo_open = emac_ndo_open,
.ndo_stop = emac_ndo_stop,
@@ -737,6 +780,7 @@ static const struct net_device_ops emac_netdev_ops = {
.ndo_eth_ioctl = icssg_ndo_ioctl,
.ndo_get_stats64 = icssg_ndo_get_stats64,
.ndo_get_phys_port_name = icssg_ndo_get_phys_port_name,
+ .ndo_set_features = emac_ndo_set_features,
};
static int prueth_netdev_init(struct prueth *prueth,
@@ -865,6 +909,7 @@ static int prueth_netdev_init(struct prueth *prueth,
ndev->ethtool_ops = &icssg_ethtool_ops;
ndev->hw_features = NETIF_F_SG;
ndev->features = ndev->hw_features;
+ ndev->hw_features |= NETIF_PRUETH_HSR_OFFLOAD_FEATURES;
netif_napi_add(ndev, &emac->napi_rx, icssg_napi_rx_poll);
hrtimer_init(&emac->rx_hrtimer, CLOCK_MONOTONIC,
@@ -953,7 +998,7 @@ static void prueth_emac_restart(struct prueth *prueth)
netif_device_attach(emac1->ndev);
}
-static void icssg_enable_switch_mode(struct prueth *prueth)
+static void icssg_change_mode(struct prueth *prueth)
{
struct prueth_emac *emac;
int mac;
@@ -973,8 +1018,13 @@ static void icssg_enable_switch_mode(struct prueth *prueth)
BIT(emac->port_id) | DEFAULT_PORT_MASK,
BIT(emac->port_id) | DEFAULT_UNTAG_MASK,
true);
+ if (prueth->is_hsr_offload_mode)
+ icssg_vtbl_modify(emac, DEFAULT_VID,
+ DEFAULT_PORT_MASK,
+ DEFAULT_UNTAG_MASK, true);
icssg_set_pvid(prueth, emac->port_vlan, emac->port_id);
- icssg_set_port_state(emac, ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE);
+ if (prueth->is_switch_mode)
+ icssg_set_port_state(emac, ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE);
}
}
}
@@ -1012,7 +1062,7 @@ static int prueth_netdevice_port_link(struct net_device *ndev,
prueth->is_switch_mode = true;
prueth->default_vlan = 1;
emac->port_vlan = prueth->default_vlan;
- icssg_enable_switch_mode(prueth);
+ icssg_change_mode(prueth);
}
}
@@ -1040,6 +1090,61 @@ static void prueth_netdevice_port_unlink(struct net_device *ndev)
prueth->hw_bridge_dev = NULL;
}
+static int prueth_hsr_port_link(struct net_device *ndev)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth *prueth = emac->prueth;
+ struct prueth_emac *emac0;
+ struct prueth_emac *emac1;
+
+ emac0 = prueth->emac[PRUETH_MAC0];
+ emac1 = prueth->emac[PRUETH_MAC1];
+
+ if (prueth->is_switch_mode)
+ return -EOPNOTSUPP;
+
+ prueth->hsr_members |= BIT(emac->port_id);
+ if (!prueth->is_hsr_offload_mode) {
+ if (prueth->hsr_members & BIT(PRUETH_PORT_MII0) &&
+ prueth->hsr_members & BIT(PRUETH_PORT_MII1)) {
+ if (!(emac0->ndev->features &
+ NETIF_PRUETH_HSR_OFFLOAD_FEATURES) &&
+ !(emac1->ndev->features &
+ NETIF_PRUETH_HSR_OFFLOAD_FEATURES))
+ return -EOPNOTSUPP;
+ prueth->is_hsr_offload_mode = true;
+ prueth->default_vlan = 1;
+ emac0->port_vlan = prueth->default_vlan;
+ emac1->port_vlan = prueth->default_vlan;
+ icssg_change_mode(prueth);
+ netdev_dbg(ndev, "Enabling HSR offload mode\n");
+ }
+ }
+
+ return 0;
+}
+
+static void prueth_hsr_port_unlink(struct net_device *ndev)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth *prueth = emac->prueth;
+ struct prueth_emac *emac0;
+ struct prueth_emac *emac1;
+
+ emac0 = prueth->emac[PRUETH_MAC0];
+ emac1 = prueth->emac[PRUETH_MAC1];
+
+ prueth->hsr_members &= ~BIT(emac->port_id);
+ if (prueth->is_hsr_offload_mode) {
+ prueth->is_hsr_offload_mode = false;
+ emac0->port_vlan = 0;
+ emac1->port_vlan = 0;
+ prueth->hsr_dev = NULL;
+ prueth_emac_restart(prueth);
+ netdev_dbg(ndev, "Disabling HSR Offload mode\n");
+ }
+}
+
/* netdev notifier */
static int prueth_netdevice_event(struct notifier_block *unused,
unsigned long event, void *ptr)
@@ -1047,6 +1152,8 @@ static int prueth_netdevice_event(struct notifier_block *unused,
struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(ptr);
struct net_device *ndev = netdev_notifier_info_to_dev(ptr);
struct netdev_notifier_changeupper_info *info;
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth *prueth = emac->prueth;
int ret = NOTIFY_DONE;
if (ndev->netdev_ops != &emac_netdev_ops)
@@ -1056,6 +1163,25 @@ static int prueth_netdevice_event(struct notifier_block *unused,
case NETDEV_CHANGEUPPER:
info = ptr;
+ if ((ndev->features & NETIF_PRUETH_HSR_OFFLOAD_FEATURES) &&
+ is_hsr_master(info->upper_dev)) {
+ if (info->linking) {
+ if (!prueth->hsr_dev) {
+ prueth->hsr_dev = info->upper_dev;
+ icssg_class_set_host_mac_addr(prueth->miig_rt,
+ prueth->hsr_dev->dev_addr);
+ } else {
+ if (prueth->hsr_dev != info->upper_dev) {
+ netdev_dbg(ndev, "Both interfaces must be linked to same upper device\n");
+ return -EOPNOTSUPP;
+ }
+ }
+ prueth_hsr_port_link(ndev);
+ } else {
+ prueth_hsr_port_unlink(ndev);
+ }
+ }
+
if (netif_is_bridge_master(info->upper_dev)) {
if (info->linking)
ret = prueth_netdevice_port_link(ndev, info->upper_dev, extack);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index 786bd1ba34ab..a4b025fae797 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -243,11 +243,14 @@ struct icssg_firmwares {
* @iep1: pointer to IEP1 device
* @vlan_tbl: VLAN-FID table pointer
* @hw_bridge_dev: pointer to HW bridge net device
+ * @hsr_dev: pointer to the HSR net device
* @br_members: bitmask of bridge member ports
+ * @hsr_members: bitmask of hsr member ports
* @prueth_netdevice_nb: netdevice notifier block
* @prueth_switchdev_nb: switchdev notifier block
* @prueth_switchdev_bl_nb: switchdev blocking notifier block
* @is_switch_mode: flag to indicate if device is in Switch mode
+ * @is_hsr_offload_mode: flag to indicate if device is in hsr offload mode
* @is_switchmode_supported: indicates platform support for switch mode
* @switch_id: ID for mapping switch ports to bridge
* @default_vlan: Default VLAN for host
@@ -279,11 +282,14 @@ struct prueth {
struct prueth_vlan_tbl *vlan_tbl;
struct net_device *hw_bridge_dev;
+ struct net_device *hsr_dev;
u8 br_members;
+ u8 hsr_members;
struct notifier_block prueth_netdevice_nb;
struct notifier_block prueth_switchdev_nb;
struct notifier_block prueth_switchdev_bl_nb;
bool is_switch_mode;
+ bool is_hsr_offload_mode;
bool is_switchmode_supported;
unsigned char switch_id[MAX_PHYS_ITEM_ID_LEN];
int default_vlan;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next v5 4/5] net: ti: icssg-prueth: Enable HSR Tx duplication, Tx Tag and Rx Tag offload
2024-09-06 11:15 [PATCH net-next v5 0/5] Introduce HSR offload support for ICSSG MD Danish Anwar
` (2 preceding siblings ...)
2024-09-06 11:15 ` [PATCH net-next v5 3/5] net: ti: icssg-prueth: Add support for HSR frame forward offload MD Danish Anwar
@ 2024-09-06 11:15 ` MD Danish Anwar
2024-09-10 17:38 ` Roger Quadros
2024-09-06 11:15 ` [PATCH net-next v5 5/5] net: ti: icssg-prueth: Add multicast filtering support in HSR mode MD Danish Anwar
4 siblings, 1 reply; 13+ messages in thread
From: MD Danish Anwar @ 2024-09-06 11:15 UTC (permalink / raw)
To: robh, jan.kiszka, dan.carpenter, saikrishnag, andrew,
javier.carrasco.cruz, jacob.e.keller, diogo.ivo, horms,
richardcochran, pabeni, kuba, edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
Roger Quadros, danishanwar, Ravi Gunasekaran
From: Ravi Gunasekaran <r-gunasekaran@ti.com>
The HSR stack allows to offload its Tx packet duplication functionality to
the hardware. Enable this offloading feature for ICSSG driver. Add support
to offload HSR Tx Tag Insertion and Rx Tag Removal and duplicate discard.
Inorder to enable hsr-tag-ins-offload, hsr-dup-offload must also be enabled
as these are tightly coupled in the firmware implementation.
Duplicate discard is done as part of RX tag removal and it is
done by the firmware. When driver sends the r30 command
ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE, firmware does RX tag removal as well as
duplicate discard.
Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
drivers/net/ethernet/ti/icssg/icssg_common.c | 18 ++++++++++---
drivers/net/ethernet/ti/icssg/icssg_config.c | 4 ++-
drivers/net/ethernet/ti/icssg/icssg_config.h | 2 ++
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 28 +++++++++++++++++++-
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 3 +++
5 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index b9d8a93d1680..fdebeb2f84e0 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -660,14 +660,15 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
{
struct cppi5_host_desc_t *first_desc, *next_desc, *cur_desc;
struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth *prueth = emac->prueth;
struct netdev_queue *netif_txq;
struct prueth_tx_chn *tx_chn;
dma_addr_t desc_dma, buf_dma;
+ u32 pkt_len, dst_tag_id;
int i, ret = 0, q_idx;
bool in_tx_ts = 0;
int tx_ts_cookie;
void **swdata;
- u32 pkt_len;
u32 *epib;
pkt_len = skb_headlen(skb);
@@ -712,9 +713,20 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
/* set dst tag to indicate internal qid at the firmware which is at
* bit8..bit15. bit0..bit7 indicates port num for directed
- * packets in case of switch mode operation
+ * packets in case of switch mode operation and port num 0
+ * for undirected packets in case of HSR offload mode
*/
- cppi5_desc_set_tags_ids(&first_desc->hdr, 0, (emac->port_id | (q_idx << 8)));
+ dst_tag_id = emac->port_id | (q_idx << 8);
+
+ if (prueth->is_hsr_offload_mode &&
+ (ndev->features & NETIF_F_HW_HSR_DUP))
+ dst_tag_id = PRUETH_UNDIRECTED_PKT_DST_TAG;
+
+ if (prueth->is_hsr_offload_mode &&
+ (ndev->features & NETIF_F_HW_HSR_TAG_INS))
+ epib[1] |= PRUETH_UNDIRECTED_PKT_TAG_INS;
+
+ cppi5_desc_set_tags_ids(&first_desc->hdr, 0, dst_tag_id);
k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
cppi5_hdesc_attach_buf(first_desc, buf_dma, pkt_len, buf_dma, pkt_len);
swdata = cppi5_hdesc_get_swdata(first_desc);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
index 7b2e6c192ff3..72ace151d8e9 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
@@ -531,7 +531,9 @@ static const struct icssg_r30_cmd emac_r32_bitmask[] = {
{{EMAC_NONE, 0xffff4000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx ENABLE*/
{{EMAC_NONE, 0xbfff0000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx DISABLE*/
{{0xffff0010, EMAC_NONE, 0xffff0010, EMAC_NONE}}, /* VLAN AWARE*/
- {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}} /* VLAN UNWARE*/
+ {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}}, /* VLAN UNWARE*/
+ {{0xffff2000, EMAC_NONE, EMAC_NONE, EMAC_NONE}}, /* HSR_RX_OFFLOAD_ENABLE */
+ {{0xdfff0000, EMAC_NONE, EMAC_NONE, EMAC_NONE}} /* HSR_RX_OFFLOAD_DISABLE */
};
int icssg_set_port_state(struct prueth_emac *emac,
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
index 1ac60283923b..92c2deaa3068 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
@@ -80,6 +80,8 @@ enum icssg_port_state_cmd {
ICSSG_EMAC_PORT_PREMPT_TX_DISABLE,
ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE,
ICSSG_EMAC_PORT_VLAN_AWARE_DISABLE,
+ ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE,
+ ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE,
ICSSG_EMAC_PORT_MAX_COMMANDS
};
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 676168d6fded..9af06454ba64 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -41,7 +41,10 @@
#define DEFAULT_PORT_MASK 1
#define DEFAULT_UNTAG_MASK 1
-#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES NETIF_F_HW_HSR_FWD
+#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES (NETIF_F_HW_HSR_FWD | \
+ NETIF_F_HW_HSR_DUP | \
+ NETIF_F_HW_HSR_TAG_INS | \
+ NETIF_F_HW_HSR_TAG_RM)
/* CTRLMMR_ICSSG_RGMII_CTRL register bits */
#define ICSSG_CTRL_RGMII_ID_MODE BIT(24)
@@ -758,6 +761,21 @@ static void emac_change_hsr_feature(struct net_device *ndev,
}
}
+static netdev_features_t emac_ndo_fix_features(struct net_device *ndev,
+ netdev_features_t features)
+{
+ /* In order to enable hsr tag insertion offload, hsr dup offload must
+ * also be enabled as these two are tightly coupled in firmware
+ * implementation.
+ */
+ if (features & NETIF_F_HW_HSR_TAG_INS)
+ features |= NETIF_F_HW_HSR_DUP;
+ else
+ features &= ~NETIF_F_HW_HSR_DUP;
+
+ return features;
+}
+
static int emac_ndo_set_features(struct net_device *ndev,
netdev_features_t features)
{
@@ -780,6 +798,7 @@ static const struct net_device_ops emac_netdev_ops = {
.ndo_eth_ioctl = icssg_ndo_ioctl,
.ndo_get_stats64 = icssg_ndo_get_stats64,
.ndo_get_phys_port_name = icssg_ndo_get_phys_port_name,
+ .ndo_fix_features = emac_ndo_fix_features,
.ndo_set_features = emac_ndo_set_features,
};
@@ -1007,6 +1026,13 @@ static void icssg_change_mode(struct prueth *prueth)
for (mac = PRUETH_MAC0; mac < PRUETH_NUM_MACS; mac++) {
emac = prueth->emac[mac];
+ if (prueth->is_hsr_offload_mode) {
+ if (emac->ndev->features & NETIF_F_HW_HSR_TAG_RM)
+ icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE);
+ else
+ icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE);
+ }
+
if (netif_running(emac->ndev)) {
icssg_fdb_add_del(emac, eth_stp_addr, prueth->default_vlan,
ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index a4b025fae797..bba6da2e6bd8 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -59,6 +59,9 @@
#define IEP_DEFAULT_CYCLE_TIME_NS 1000000 /* 1 ms */
+#define PRUETH_UNDIRECTED_PKT_DST_TAG 0
+#define PRUETH_UNDIRECTED_PKT_TAG_INS BIT(30)
+
/* Firmware status codes */
#define ICSS_HS_FW_READY 0x55555555
#define ICSS_HS_FW_DEAD 0xDEAD0000 /* lower 16 bits contain error code */
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next v5 5/5] net: ti: icssg-prueth: Add multicast filtering support in HSR mode
2024-09-06 11:15 [PATCH net-next v5 0/5] Introduce HSR offload support for ICSSG MD Danish Anwar
` (3 preceding siblings ...)
2024-09-06 11:15 ` [PATCH net-next v5 4/5] net: ti: icssg-prueth: Enable HSR Tx duplication, Tx Tag and Rx Tag offload MD Danish Anwar
@ 2024-09-06 11:15 ` MD Danish Anwar
4 siblings, 0 replies; 13+ messages in thread
From: MD Danish Anwar @ 2024-09-06 11:15 UTC (permalink / raw)
To: robh, jan.kiszka, dan.carpenter, saikrishnag, andrew,
javier.carrasco.cruz, jacob.e.keller, diogo.ivo, horms,
richardcochran, pabeni, kuba, edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
Roger Quadros, danishanwar
Add support for multicast filtering in HSR mode
Reviewed-by: Roger Quadros <rogerq@kernel.org>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 42 +++++++++++++++++++-
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 9af06454ba64..a8200ee79a89 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -492,6 +492,36 @@ static int icssg_prueth_del_mcast(struct net_device *ndev, const u8 *addr)
return 0;
}
+static int icssg_prueth_hsr_add_mcast(struct net_device *ndev, const u8 *addr)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth *prueth = emac->prueth;
+
+ icssg_fdb_add_del(emac, addr, prueth->default_vlan,
+ ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
+ ICSSG_FDB_ENTRY_P1_MEMBERSHIP |
+ ICSSG_FDB_ENTRY_P2_MEMBERSHIP |
+ ICSSG_FDB_ENTRY_BLOCK, true);
+
+ icssg_vtbl_modify(emac, emac->port_vlan, BIT(emac->port_id),
+ BIT(emac->port_id), true);
+ return 0;
+}
+
+static int icssg_prueth_hsr_del_mcast(struct net_device *ndev, const u8 *addr)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth *prueth = emac->prueth;
+
+ icssg_fdb_add_del(emac, addr, prueth->default_vlan,
+ ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
+ ICSSG_FDB_ENTRY_P1_MEMBERSHIP |
+ ICSSG_FDB_ENTRY_P2_MEMBERSHIP |
+ ICSSG_FDB_ENTRY_BLOCK, false);
+
+ return 0;
+}
+
/**
* emac_ndo_open - EMAC device open
* @ndev: network adapter device
@@ -652,7 +682,10 @@ static int emac_ndo_stop(struct net_device *ndev)
icssg_class_disable(prueth->miig_rt, prueth_emac_slice(emac));
- __dev_mc_unsync(ndev, icssg_prueth_del_mcast);
+ if (emac->prueth->is_hsr_offload_mode)
+ __dev_mc_unsync(ndev, icssg_prueth_hsr_del_mcast);
+ else
+ __dev_mc_unsync(ndev, icssg_prueth_del_mcast);
atomic_set(&emac->tdown_cnt, emac->tx_ch_num);
/* ensure new tdown_cnt value is visible */
@@ -730,7 +763,12 @@ static void emac_ndo_set_rx_mode_work(struct work_struct *work)
return;
}
- __dev_mc_sync(ndev, icssg_prueth_add_mcast, icssg_prueth_del_mcast);
+ if (emac->prueth->is_hsr_offload_mode)
+ __dev_mc_sync(ndev, icssg_prueth_hsr_add_mcast,
+ icssg_prueth_hsr_del_mcast);
+ else
+ __dev_mc_sync(ndev, icssg_prueth_add_mcast,
+ icssg_prueth_del_mcast);
}
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v5 3/5] net: ti: icssg-prueth: Add support for HSR frame forward offload
2024-09-06 11:15 ` [PATCH net-next v5 3/5] net: ti: icssg-prueth: Add support for HSR frame forward offload MD Danish Anwar
@ 2024-09-10 12:12 ` Roger Quadros
2024-09-10 13:13 ` Anwar, Md Danish
0 siblings, 1 reply; 13+ messages in thread
From: Roger Quadros @ 2024-09-10 12:12 UTC (permalink / raw)
To: MD Danish Anwar, robh, jan.kiszka, dan.carpenter, saikrishnag,
andrew, javier.carrasco.cruz, jacob.e.keller, diogo.ivo, horms,
richardcochran, pabeni, kuba, edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra
Hi,
On 06/09/2024 14:15, MD Danish Anwar wrote:
> Add support for offloading HSR port-to-port frame forward to hardware.
> When the slave interfaces are added to the HSR interface, the PRU cores
> will be stopped and ICSSG HSR firmwares will be loaded to them.
>
> Similarly, when HSR interface is deleted, the PRU cores will be
> restarted and the last used firmwares will be reloaded. PRUeth
> interfaces will be back to the last used mode.
>
> This commit also renames some APIs that are common between switch and
> hsr mode with '_fw_offload' suffix.
>
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
> .../net/ethernet/ti/icssg/icssg_classifier.c | 1 +
> drivers/net/ethernet/ti/icssg/icssg_config.c | 18 +--
> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 132 +++++++++++++++++-
> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 6 +
> 4 files changed, 145 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_classifier.c b/drivers/net/ethernet/ti/icssg/icssg_classifier.c
> index 9ec504d976d6..833ca86d0b71 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_classifier.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_classifier.c
<snip>
> +static void emac_change_hsr_feature(struct net_device *ndev,
> + netdev_features_t features,
> + u64 hsr_feature)
> +{
> + netdev_features_t changed = ndev->features ^ features;
> +
> + if (changed & hsr_feature) {
> + if (features & hsr_feature)
> + ndev->features |= hsr_feature;
> + else
> + ndev->features &= ~hsr_feature;
You are not supposed to change ndev->features here.
From
"https://www.kernel.org/doc/Documentation/networking/netdev-features.txt"
"
* ndo_set_features:
Hardware should be reconfigured to match passed feature set. The set
should not be altered unless some error condition happens that can't
be reliably detected in ndo_fix_features. In this case, the callback
should update netdev->features to match resulting hardware state.
Errors returned are not (and cannot be) propagated anywhere except dmesg.
(Note: successful return is zero, >0 means silent error.)"
This means only in
> + }
> +}
> +
> +static int emac_ndo_set_features(struct net_device *ndev,
> + netdev_features_t features)
> +{
> + emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_FWD);
> + emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_DUP);
> + emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_TAG_INS);
> + emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_TAG_RM);
I don't understand this part.
As you are not changing hardware state in ndo_set_features, I'm not sure why
you even need ndo_set_features callback.
You didn't take my feedback about using ndo_fix_features().
Please read this.
https://www.kernel.org/doc/Documentation/networking/netdev-features.txt
"Part II: Controlling enabled features"
"Part III: Implementation hints"
Also look at how _netdev_update_features() works and calls ndo_fix_features()
and ndo_set_features()
https://elixir.bootlin.com/linux/v6.11-rc7/source/net/core/dev.c#L10023
> +
> + return 0;
> +}
> +
> static const struct net_device_ops emac_netdev_ops = {
> .ndo_open = emac_ndo_open,
> .ndo_stop = emac_ndo_stop,
> @@ -737,6 +780,7 @@ static const struct net_device_ops emac_netdev_ops = {
> .ndo_eth_ioctl = icssg_ndo_ioctl,
> .ndo_get_stats64 = icssg_ndo_get_stats64,
> .ndo_get_phys_port_name = icssg_ndo_get_phys_port_name,
> + .ndo_set_features = emac_ndo_set_features,
> };
<snip>
--
cheers,
-roger
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v5 3/5] net: ti: icssg-prueth: Add support for HSR frame forward offload
2024-09-10 12:12 ` Roger Quadros
@ 2024-09-10 13:13 ` Anwar, Md Danish
0 siblings, 0 replies; 13+ messages in thread
From: Anwar, Md Danish @ 2024-09-10 13:13 UTC (permalink / raw)
To: Roger Quadros, MD Danish Anwar, robh, jan.kiszka, dan.carpenter,
saikrishnag, andrew, javier.carrasco.cruz, jacob.e.keller,
diogo.ivo, horms, richardcochran, pabeni, kuba, edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra
Hi Roger,
On 9/10/2024 5:42 PM, Roger Quadros wrote:
> Hi,
>
> On 06/09/2024 14:15, MD Danish Anwar wrote:
>> Add support for offloading HSR port-to-port frame forward to hardware.
>> When the slave interfaces are added to the HSR interface, the PRU cores
>> will be stopped and ICSSG HSR firmwares will be loaded to them.
>>
>> Similarly, when HSR interface is deleted, the PRU cores will be
>> restarted and the last used firmwares will be reloaded. PRUeth
>> interfaces will be back to the last used mode.
>>
>> This commit also renames some APIs that are common between switch and
>> hsr mode with '_fw_offload' suffix.
>>
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> ---
>> .../net/ethernet/ti/icssg/icssg_classifier.c | 1 +
>> drivers/net/ethernet/ti/icssg/icssg_config.c | 18 +--
>> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 132 +++++++++++++++++-
>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 6 +
>> 4 files changed, 145 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_classifier.c b/drivers/net/ethernet/ti/icssg/icssg_classifier.c
>> index 9ec504d976d6..833ca86d0b71 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_classifier.c
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_classifier.c
>
> <snip>
>
>> +static void emac_change_hsr_feature(struct net_device *ndev,
>> + netdev_features_t features,
>> + u64 hsr_feature)
>> +{
>> + netdev_features_t changed = ndev->features ^ features;
>> +
>> + if (changed & hsr_feature) {
>> + if (features & hsr_feature)
>> + ndev->features |= hsr_feature;
>> + else
>> + ndev->features &= ~hsr_feature;
>
> You are not supposed to change ndev->features here.
>
> From
> "https://www.kernel.org/doc/Documentation/networking/netdev-features.txt"
> "
> * ndo_set_features:
>
> Hardware should be reconfigured to match passed feature set. The set
> should not be altered unless some error condition happens that can't
> be reliably detected in ndo_fix_features. In this case, the callback
> should update netdev->features to match resulting hardware state.
> Errors returned are not (and cannot be) propagated anywhere except dmesg.
> (Note: successful return is zero, >0 means silent error.)"
>
> This means only in
>
>> + }
>> +}
>> +
>> +static int emac_ndo_set_features(struct net_device *ndev,
>> + netdev_features_t features)
>> +{
>> + emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_FWD);
>> + emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_DUP);
>> + emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_TAG_INS);
>> + emac_change_hsr_feature(ndev, features, NETIF_F_HW_HSR_TAG_RM);
>
> I don't understand this part.
>
> As you are not changing hardware state in ndo_set_features, I'm not sure why
> you even need ndo_set_features callback.
>
We don't need to do any hardware configuration when the feature is
changed. Instead certain APIs will do certain actions based on which HSR
related feature is set in ndev->features. I agree this makes the whole
API redundant. Previously in v3
(https://lore.kernel.org/all/20240828091901.3120935-4-danishanwar@ti.com/)
also this API was essentially just changing ndev->features when hsr
change was requested.
+static int emac_ndo_set_features(struct net_device *ndev,
+ netdev_features_t features)
+{
...
+
+ if (hsr_change_request)
+ ndev->features = features;
+
+ return 0;
+}
I think it would be better to drop this call back in the driver as no
action is needed in the hardware based on what feature is set here.
> You didn't take my feedback about using ndo_fix_features().
>
Roger, I have taken your feedback regarding ndo_fix_features() and
implemented it in the next patch (patch 4/5
https://lore.kernel.org/all/20240906111538.1259418-5-danishanwar@ti.com/).
Please have a look at that patch also. The features that have
dependencies that can be addressed in ndo_fix_features, are not
introduced in this patch so I have not introduced the function here. It
is getting introduced in the next patch.
> Please read this.
> https://www.kernel.org/doc/Documentation/networking/netdev-features.txt
> "Part II: Controlling enabled features"
> "Part III: Implementation hints"
>
> Also look at how _netdev_update_features() works and calls ndo_fix_features()
> and ndo_set_features()
>
Thanks for this. I checked that. Based on how _netdev_update_features()
is implemented, I don't think there is any need of ndo_set_features.
Whatever hardware dependencies are related to these HSR features, can be
taken care by ndo_fix_features.
Please let me know how does this look to you.
> https://elixir.bootlin.com/linux/v6.11-rc7/source/net/core/dev.c#L10023
>
>> +
>> + return 0;
>> +}
>> +
>> static const struct net_device_ops emac_netdev_ops = {
>> .ndo_open = emac_ndo_open,
>> .ndo_stop = emac_ndo_stop,
>> @@ -737,6 +780,7 @@ static const struct net_device_ops emac_netdev_ops = {
>> .ndo_eth_ioctl = icssg_ndo_ioctl,
>> .ndo_get_stats64 = icssg_ndo_get_stats64,
>> .ndo_get_phys_port_name = icssg_ndo_get_phys_port_name,
>> + .ndo_set_features = emac_ndo_set_features,
>> };
>
> <snip>
>
--
Thanks and Regards,
Md Danish Anwar
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v5 4/5] net: ti: icssg-prueth: Enable HSR Tx duplication, Tx Tag and Rx Tag offload
2024-09-06 11:15 ` [PATCH net-next v5 4/5] net: ti: icssg-prueth: Enable HSR Tx duplication, Tx Tag and Rx Tag offload MD Danish Anwar
@ 2024-09-10 17:38 ` Roger Quadros
2024-09-10 17:52 ` Anwar, Md Danish
0 siblings, 1 reply; 13+ messages in thread
From: Roger Quadros @ 2024-09-10 17:38 UTC (permalink / raw)
To: MD Danish Anwar, robh, jan.kiszka, dan.carpenter, saikrishnag,
andrew, javier.carrasco.cruz, jacob.e.keller, diogo.ivo, horms,
richardcochran, pabeni, kuba, edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
Ravi Gunasekaran
On 06/09/2024 14:15, MD Danish Anwar wrote:
> From: Ravi Gunasekaran <r-gunasekaran@ti.com>
>
> The HSR stack allows to offload its Tx packet duplication functionality to
> the hardware. Enable this offloading feature for ICSSG driver. Add support
> to offload HSR Tx Tag Insertion and Rx Tag Removal and duplicate discard.
>
> Inorder to enable hsr-tag-ins-offload, hsr-dup-offload must also be enabled
"In order"
> as these are tightly coupled in the firmware implementation.
>
> Duplicate discard is done as part of RX tag removal and it is
> done by the firmware. When driver sends the r30 command
> ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE, firmware does RX tag removal as well as
> duplicate discard.
>
> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
> drivers/net/ethernet/ti/icssg/icssg_common.c | 18 ++++++++++---
> drivers/net/ethernet/ti/icssg/icssg_config.c | 4 ++-
> drivers/net/ethernet/ti/icssg/icssg_config.h | 2 ++
> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 28 +++++++++++++++++++-
> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 3 +++
> 5 files changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index b9d8a93d1680..fdebeb2f84e0 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
> @@ -660,14 +660,15 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
> {
> struct cppi5_host_desc_t *first_desc, *next_desc, *cur_desc;
> struct prueth_emac *emac = netdev_priv(ndev);
> + struct prueth *prueth = emac->prueth;
> struct netdev_queue *netif_txq;
> struct prueth_tx_chn *tx_chn;
> dma_addr_t desc_dma, buf_dma;
> + u32 pkt_len, dst_tag_id;
> int i, ret = 0, q_idx;
> bool in_tx_ts = 0;
> int tx_ts_cookie;
> void **swdata;
> - u32 pkt_len;
> u32 *epib;
>
> pkt_len = skb_headlen(skb);
> @@ -712,9 +713,20 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
>
> /* set dst tag to indicate internal qid at the firmware which is at
> * bit8..bit15. bit0..bit7 indicates port num for directed
> - * packets in case of switch mode operation
> + * packets in case of switch mode operation and port num 0
> + * for undirected packets in case of HSR offload mode
> */
> - cppi5_desc_set_tags_ids(&first_desc->hdr, 0, (emac->port_id | (q_idx << 8)));
> + dst_tag_id = emac->port_id | (q_idx << 8);
> +
> + if (prueth->is_hsr_offload_mode &&
> + (ndev->features & NETIF_F_HW_HSR_DUP))
> + dst_tag_id = PRUETH_UNDIRECTED_PKT_DST_TAG;
> +
> + if (prueth->is_hsr_offload_mode &&
> + (ndev->features & NETIF_F_HW_HSR_TAG_INS))
> + epib[1] |= PRUETH_UNDIRECTED_PKT_TAG_INS;
> +
> + cppi5_desc_set_tags_ids(&first_desc->hdr, 0, dst_tag_id);
> k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
> cppi5_hdesc_attach_buf(first_desc, buf_dma, pkt_len, buf_dma, pkt_len);
> swdata = cppi5_hdesc_get_swdata(first_desc);
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
> index 7b2e6c192ff3..72ace151d8e9 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
> @@ -531,7 +531,9 @@ static const struct icssg_r30_cmd emac_r32_bitmask[] = {
> {{EMAC_NONE, 0xffff4000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx ENABLE*/
> {{EMAC_NONE, 0xbfff0000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx DISABLE*/
> {{0xffff0010, EMAC_NONE, 0xffff0010, EMAC_NONE}}, /* VLAN AWARE*/
> - {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}} /* VLAN UNWARE*/
> + {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}}, /* VLAN UNWARE*/
> + {{0xffff2000, EMAC_NONE, EMAC_NONE, EMAC_NONE}}, /* HSR_RX_OFFLOAD_ENABLE */
> + {{0xdfff0000, EMAC_NONE, EMAC_NONE, EMAC_NONE}} /* HSR_RX_OFFLOAD_DISABLE */
> };
>
> int icssg_set_port_state(struct prueth_emac *emac,
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
> index 1ac60283923b..92c2deaa3068 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
> @@ -80,6 +80,8 @@ enum icssg_port_state_cmd {
> ICSSG_EMAC_PORT_PREMPT_TX_DISABLE,
> ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE,
> ICSSG_EMAC_PORT_VLAN_AWARE_DISABLE,
> + ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE,
> + ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE,
> ICSSG_EMAC_PORT_MAX_COMMANDS
> };
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index 676168d6fded..9af06454ba64 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -41,7 +41,10 @@
> #define DEFAULT_PORT_MASK 1
> #define DEFAULT_UNTAG_MASK 1
>
> -#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES NETIF_F_HW_HSR_FWD
> +#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES (NETIF_F_HW_HSR_FWD | \
> + NETIF_F_HW_HSR_DUP | \
> + NETIF_F_HW_HSR_TAG_INS | \
> + NETIF_F_HW_HSR_TAG_RM)
>
> /* CTRLMMR_ICSSG_RGMII_CTRL register bits */
> #define ICSSG_CTRL_RGMII_ID_MODE BIT(24)
> @@ -758,6 +761,21 @@ static void emac_change_hsr_feature(struct net_device *ndev,
> }
> }
>
> +static netdev_features_t emac_ndo_fix_features(struct net_device *ndev,
> + netdev_features_t features)
> +{
> + /* In order to enable hsr tag insertion offload, hsr dup offload must
> + * also be enabled as these two are tightly coupled in firmware
> + * implementation.
> + */
> + if (features & NETIF_F_HW_HSR_TAG_INS)
> + features |= NETIF_F_HW_HSR_DUP;
What if only NETIF_F_HW_HSR_DUP was set? Don't you have to set NETIF_F_HW_HSR_TAG_INS as well?
> + else
> + features &= ~NETIF_F_HW_HSR_DUP;
what if NETIF_F_HW_HSR_DUP was still set?
I think you need to write a logic like follows.
if both are already cleared in ndev->features and any one is set in features you set both in features.
if both are already set in ndev->features and any one is cleared in features you clear both in features.
is this reasonable?
> +
> + return features;
> +}
> +
> static int emac_ndo_set_features(struct net_device *ndev,
> netdev_features_t features)
> {
> @@ -780,6 +798,7 @@ static const struct net_device_ops emac_netdev_ops = {
> .ndo_eth_ioctl = icssg_ndo_ioctl,
> .ndo_get_stats64 = icssg_ndo_get_stats64,
> .ndo_get_phys_port_name = icssg_ndo_get_phys_port_name,
> + .ndo_fix_features = emac_ndo_fix_features,
> .ndo_set_features = emac_ndo_set_features,
> };
>
> @@ -1007,6 +1026,13 @@ static void icssg_change_mode(struct prueth *prueth)
>
> for (mac = PRUETH_MAC0; mac < PRUETH_NUM_MACS; mac++) {
> emac = prueth->emac[mac];
> + if (prueth->is_hsr_offload_mode) {
> + if (emac->ndev->features & NETIF_F_HW_HSR_TAG_RM)
> + icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE);
> + else
> + icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE);
> + }
> +
> if (netif_running(emac->ndev)) {
> icssg_fdb_add_del(emac, eth_stp_addr, prueth->default_vlan,
> ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> index a4b025fae797..bba6da2e6bd8 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> @@ -59,6 +59,9 @@
>
> #define IEP_DEFAULT_CYCLE_TIME_NS 1000000 /* 1 ms */
>
> +#define PRUETH_UNDIRECTED_PKT_DST_TAG 0
> +#define PRUETH_UNDIRECTED_PKT_TAG_INS BIT(30)
> +
> /* Firmware status codes */
> #define ICSS_HS_FW_READY 0x55555555
> #define ICSS_HS_FW_DEAD 0xDEAD0000 /* lower 16 bits contain error code */
--
cheers,
-roger
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v5 4/5] net: ti: icssg-prueth: Enable HSR Tx duplication, Tx Tag and Rx Tag offload
2024-09-10 17:38 ` Roger Quadros
@ 2024-09-10 17:52 ` Anwar, Md Danish
2024-09-10 18:06 ` Roger Quadros
0 siblings, 1 reply; 13+ messages in thread
From: Anwar, Md Danish @ 2024-09-10 17:52 UTC (permalink / raw)
To: Roger Quadros, MD Danish Anwar, robh, jan.kiszka, dan.carpenter,
saikrishnag, andrew, javier.carrasco.cruz, jacob.e.keller,
diogo.ivo, horms, richardcochran, pabeni, kuba, edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
Ravi Gunasekaran
Hi Roger,
On 9/10/2024 11:08 PM, Roger Quadros wrote:
>
>
> On 06/09/2024 14:15, MD Danish Anwar wrote:
>> From: Ravi Gunasekaran <r-gunasekaran@ti.com>
>>
>> The HSR stack allows to offload its Tx packet duplication functionality to
>> the hardware. Enable this offloading feature for ICSSG driver. Add support
>> to offload HSR Tx Tag Insertion and Rx Tag Removal and duplicate discard.
>>
>> Inorder to enable hsr-tag-ins-offload, hsr-dup-offload must also be enabled
>
> "In order"
>
>> as these are tightly coupled in the firmware implementation.
>>
>> Duplicate discard is done as part of RX tag removal and it is
>> done by the firmware. When driver sends the r30 command
>> ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE, firmware does RX tag removal as well as
>> duplicate discard.
>>
>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> ---
>> drivers/net/ethernet/ti/icssg/icssg_common.c | 18 ++++++++++---
>> drivers/net/ethernet/ti/icssg/icssg_config.c | 4 ++-
>> drivers/net/ethernet/ti/icssg/icssg_config.h | 2 ++
>> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 28 +++++++++++++++++++-
>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 3 +++
>> 5 files changed, 50 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
>> index b9d8a93d1680..fdebeb2f84e0 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
>> @@ -660,14 +660,15 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
>> {
>> struct cppi5_host_desc_t *first_desc, *next_desc, *cur_desc;
>> struct prueth_emac *emac = netdev_priv(ndev);
>> + struct prueth *prueth = emac->prueth;
>> struct netdev_queue *netif_txq;
>> struct prueth_tx_chn *tx_chn;
>> dma_addr_t desc_dma, buf_dma;
>> + u32 pkt_len, dst_tag_id;
>> int i, ret = 0, q_idx;
>> bool in_tx_ts = 0;
>> int tx_ts_cookie;
>> void **swdata;
>> - u32 pkt_len;
>> u32 *epib;
>>
>> pkt_len = skb_headlen(skb);
>> @@ -712,9 +713,20 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
>>
>> /* set dst tag to indicate internal qid at the firmware which is at
>> * bit8..bit15. bit0..bit7 indicates port num for directed
>> - * packets in case of switch mode operation
>> + * packets in case of switch mode operation and port num 0
>> + * for undirected packets in case of HSR offload mode
>> */
>> - cppi5_desc_set_tags_ids(&first_desc->hdr, 0, (emac->port_id | (q_idx << 8)));
>> + dst_tag_id = emac->port_id | (q_idx << 8);
>> +
>> + if (prueth->is_hsr_offload_mode &&
>> + (ndev->features & NETIF_F_HW_HSR_DUP))
>> + dst_tag_id = PRUETH_UNDIRECTED_PKT_DST_TAG;
>> +
>> + if (prueth->is_hsr_offload_mode &&
>> + (ndev->features & NETIF_F_HW_HSR_TAG_INS))
>> + epib[1] |= PRUETH_UNDIRECTED_PKT_TAG_INS;
>> +
>> + cppi5_desc_set_tags_ids(&first_desc->hdr, 0, dst_tag_id);
>> k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
>> cppi5_hdesc_attach_buf(first_desc, buf_dma, pkt_len, buf_dma, pkt_len);
>> swdata = cppi5_hdesc_get_swdata(first_desc);
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
>> index 7b2e6c192ff3..72ace151d8e9 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
>> @@ -531,7 +531,9 @@ static const struct icssg_r30_cmd emac_r32_bitmask[] = {
>> {{EMAC_NONE, 0xffff4000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx ENABLE*/
>> {{EMAC_NONE, 0xbfff0000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx DISABLE*/
>> {{0xffff0010, EMAC_NONE, 0xffff0010, EMAC_NONE}}, /* VLAN AWARE*/
>> - {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}} /* VLAN UNWARE*/
>> + {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}}, /* VLAN UNWARE*/
>> + {{0xffff2000, EMAC_NONE, EMAC_NONE, EMAC_NONE}}, /* HSR_RX_OFFLOAD_ENABLE */
>> + {{0xdfff0000, EMAC_NONE, EMAC_NONE, EMAC_NONE}} /* HSR_RX_OFFLOAD_DISABLE */
>> };
>>
>> int icssg_set_port_state(struct prueth_emac *emac,
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
>> index 1ac60283923b..92c2deaa3068 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
>> @@ -80,6 +80,8 @@ enum icssg_port_state_cmd {
>> ICSSG_EMAC_PORT_PREMPT_TX_DISABLE,
>> ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE,
>> ICSSG_EMAC_PORT_VLAN_AWARE_DISABLE,
>> + ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE,
>> + ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE,
>> ICSSG_EMAC_PORT_MAX_COMMANDS
>> };
>>
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>> index 676168d6fded..9af06454ba64 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>> @@ -41,7 +41,10 @@
>> #define DEFAULT_PORT_MASK 1
>> #define DEFAULT_UNTAG_MASK 1
>>
>> -#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES NETIF_F_HW_HSR_FWD
>> +#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES (NETIF_F_HW_HSR_FWD | \
>> + NETIF_F_HW_HSR_DUP | \
>> + NETIF_F_HW_HSR_TAG_INS | \
>> + NETIF_F_HW_HSR_TAG_RM)
>>
>> /* CTRLMMR_ICSSG_RGMII_CTRL register bits */
>> #define ICSSG_CTRL_RGMII_ID_MODE BIT(24)
>> @@ -758,6 +761,21 @@ static void emac_change_hsr_feature(struct net_device *ndev,
>> }
>> }
>>
>> +static netdev_features_t emac_ndo_fix_features(struct net_device *ndev,
>> + netdev_features_t features)
>> +{
>> + /* In order to enable hsr tag insertion offload, hsr dup offload must
>> + * also be enabled as these two are tightly coupled in firmware
>> + * implementation.
>> + */
>> + if (features & NETIF_F_HW_HSR_TAG_INS)
>> + features |= NETIF_F_HW_HSR_DUP;
>
> What if only NETIF_F_HW_HSR_DUP was set? Don't you have to set NETIF_F_HW_HSR_TAG_INS as well?
>
>> + else
>> + features &= ~NETIF_F_HW_HSR_DUP;
>
> what if NETIF_F_HW_HSR_DUP was still set?
>
> I think you need to write a logic like follows.
> if both are already cleared in ndev->features and any one is set in features you set both in features.
> if both are already set in ndev->features and any one is cleared in features you clear both in features.
>
> is this reasonable?
>
Yes that does sound reasonable,
How does below code look to you.
if (!(ndev->features & NETIF_F_HW_HSR_DUP) &&
!(ndev->features & NETIF_F_HW_HSR_TAG_INS))
if ((features & NETIF_F_HW_HSR_DUP) ||
(features & NETIF_F_HW_HSR_TAG_INS)) {
features |= NETIF_F_HW_HSR_DUP;
features |= NETIF_F_HW_HSR_TAG_INS;
}
if ((ndev->features & NETIF_F_HW_HSR_DUP) &&
(ndev->features & NETIF_F_HW_HSR_TAG_INS))
if (!(features & NETIF_F_HW_HSR_DUP) ||
!(features & NETIF_F_HW_HSR_TAG_INS)) {
features &= ~NETIF_F_HW_HSR_DUP;
features &= ~NETIF_F_HW_HSR_TAG_INS;
}
>> +
>> + return features;
>> +}
>> +
>> static int emac_ndo_set_features(struct net_device *ndev,
>> netdev_features_t features)
>> {
>> @@ -780,6 +798,7 @@ static const struct net_device_ops emac_netdev_ops = {
>> .ndo_eth_ioctl = icssg_ndo_ioctl,
>> .ndo_get_stats64 = icssg_ndo_get_stats64,
>> .ndo_get_phys_port_name = icssg_ndo_get_phys_port_name,
>> + .ndo_fix_features = emac_ndo_fix_features,
>> .ndo_set_features = emac_ndo_set_features,
>> };
>>
>> @@ -1007,6 +1026,13 @@ static void icssg_change_mode(struct prueth *prueth)
>>
>> for (mac = PRUETH_MAC0; mac < PRUETH_NUM_MACS; mac++) {
>> emac = prueth->emac[mac];
>> + if (prueth->is_hsr_offload_mode) {
>> + if (emac->ndev->features & NETIF_F_HW_HSR_TAG_RM)
>> + icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE);
>> + else
>> + icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE);
>> + }
>> +
>> if (netif_running(emac->ndev)) {
>> icssg_fdb_add_del(emac, eth_stp_addr, prueth->default_vlan,
>> ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> index a4b025fae797..bba6da2e6bd8 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> @@ -59,6 +59,9 @@
>>
>> #define IEP_DEFAULT_CYCLE_TIME_NS 1000000 /* 1 ms */
>>
>> +#define PRUETH_UNDIRECTED_PKT_DST_TAG 0
>> +#define PRUETH_UNDIRECTED_PKT_TAG_INS BIT(30)
>> +
>> /* Firmware status codes */
>> #define ICSS_HS_FW_READY 0x55555555
>> #define ICSS_HS_FW_DEAD 0xDEAD0000 /* lower 16 bits contain error code */
>
--
Thanks and Regards,
Md Danish Anwar
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v5 4/5] net: ti: icssg-prueth: Enable HSR Tx duplication, Tx Tag and Rx Tag offload
2024-09-10 17:52 ` Anwar, Md Danish
@ 2024-09-10 18:06 ` Roger Quadros
2024-09-10 18:27 ` Anwar, Md Danish
0 siblings, 1 reply; 13+ messages in thread
From: Roger Quadros @ 2024-09-10 18:06 UTC (permalink / raw)
To: Anwar, Md Danish, MD Danish Anwar, robh, jan.kiszka,
dan.carpenter, saikrishnag, andrew, javier.carrasco.cruz,
jacob.e.keller, diogo.ivo, horms, richardcochran, pabeni, kuba,
edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
Ravi Gunasekaran
On 10/09/2024 20:52, Anwar, Md Danish wrote:
> Hi Roger,
>
> On 9/10/2024 11:08 PM, Roger Quadros wrote:
>>
>>
>> On 06/09/2024 14:15, MD Danish Anwar wrote:
>>> From: Ravi Gunasekaran <r-gunasekaran@ti.com>
>>>
>>> The HSR stack allows to offload its Tx packet duplication functionality to
>>> the hardware. Enable this offloading feature for ICSSG driver. Add support
>>> to offload HSR Tx Tag Insertion and Rx Tag Removal and duplicate discard.
>>>
>>> Inorder to enable hsr-tag-ins-offload, hsr-dup-offload must also be enabled
>>
>> "In order"
>>
>>> as these are tightly coupled in the firmware implementation.
>>>
>>> Duplicate discard is done as part of RX tag removal and it is
>>> done by the firmware. When driver sends the r30 command
>>> ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE, firmware does RX tag removal as well as
>>> duplicate discard.
>>>
>>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>> ---
>>> drivers/net/ethernet/ti/icssg/icssg_common.c | 18 ++++++++++---
>>> drivers/net/ethernet/ti/icssg/icssg_config.c | 4 ++-
>>> drivers/net/ethernet/ti/icssg/icssg_config.h | 2 ++
>>> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 28 +++++++++++++++++++-
>>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 3 +++
>>> 5 files changed, 50 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
>>> index b9d8a93d1680..fdebeb2f84e0 100644
>>> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
>>> @@ -660,14 +660,15 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
>>> {
>>> struct cppi5_host_desc_t *first_desc, *next_desc, *cur_desc;
>>> struct prueth_emac *emac = netdev_priv(ndev);
>>> + struct prueth *prueth = emac->prueth;
>>> struct netdev_queue *netif_txq;
>>> struct prueth_tx_chn *tx_chn;
>>> dma_addr_t desc_dma, buf_dma;
>>> + u32 pkt_len, dst_tag_id;
>>> int i, ret = 0, q_idx;
>>> bool in_tx_ts = 0;
>>> int tx_ts_cookie;
>>> void **swdata;
>>> - u32 pkt_len;
>>> u32 *epib;
>>>
>>> pkt_len = skb_headlen(skb);
>>> @@ -712,9 +713,20 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
>>>
>>> /* set dst tag to indicate internal qid at the firmware which is at
>>> * bit8..bit15. bit0..bit7 indicates port num for directed
>>> - * packets in case of switch mode operation
>>> + * packets in case of switch mode operation and port num 0
>>> + * for undirected packets in case of HSR offload mode
>>> */
>>> - cppi5_desc_set_tags_ids(&first_desc->hdr, 0, (emac->port_id | (q_idx << 8)));
>>> + dst_tag_id = emac->port_id | (q_idx << 8);
>>> +
>>> + if (prueth->is_hsr_offload_mode &&
>>> + (ndev->features & NETIF_F_HW_HSR_DUP))
>>> + dst_tag_id = PRUETH_UNDIRECTED_PKT_DST_TAG;
>>> +
>>> + if (prueth->is_hsr_offload_mode &&
>>> + (ndev->features & NETIF_F_HW_HSR_TAG_INS))
>>> + epib[1] |= PRUETH_UNDIRECTED_PKT_TAG_INS;
>>> +
>>> + cppi5_desc_set_tags_ids(&first_desc->hdr, 0, dst_tag_id);
>>> k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
>>> cppi5_hdesc_attach_buf(first_desc, buf_dma, pkt_len, buf_dma, pkt_len);
>>> swdata = cppi5_hdesc_get_swdata(first_desc);
>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>> index 7b2e6c192ff3..72ace151d8e9 100644
>>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>> @@ -531,7 +531,9 @@ static const struct icssg_r30_cmd emac_r32_bitmask[] = {
>>> {{EMAC_NONE, 0xffff4000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx ENABLE*/
>>> {{EMAC_NONE, 0xbfff0000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx DISABLE*/
>>> {{0xffff0010, EMAC_NONE, 0xffff0010, EMAC_NONE}}, /* VLAN AWARE*/
>>> - {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}} /* VLAN UNWARE*/
>>> + {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}}, /* VLAN UNWARE*/
>>> + {{0xffff2000, EMAC_NONE, EMAC_NONE, EMAC_NONE}}, /* HSR_RX_OFFLOAD_ENABLE */
>>> + {{0xdfff0000, EMAC_NONE, EMAC_NONE, EMAC_NONE}} /* HSR_RX_OFFLOAD_DISABLE */
>>> };
>>>
>>> int icssg_set_port_state(struct prueth_emac *emac,
>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
>>> index 1ac60283923b..92c2deaa3068 100644
>>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
>>> @@ -80,6 +80,8 @@ enum icssg_port_state_cmd {
>>> ICSSG_EMAC_PORT_PREMPT_TX_DISABLE,
>>> ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE,
>>> ICSSG_EMAC_PORT_VLAN_AWARE_DISABLE,
>>> + ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE,
>>> + ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE,
>>> ICSSG_EMAC_PORT_MAX_COMMANDS
>>> };
>>>
>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>>> index 676168d6fded..9af06454ba64 100644
>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>>> @@ -41,7 +41,10 @@
>>> #define DEFAULT_PORT_MASK 1
>>> #define DEFAULT_UNTAG_MASK 1
>>>
>>> -#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES NETIF_F_HW_HSR_FWD
>>> +#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES (NETIF_F_HW_HSR_FWD | \
>>> + NETIF_F_HW_HSR_DUP | \
>>> + NETIF_F_HW_HSR_TAG_INS | \
>>> + NETIF_F_HW_HSR_TAG_RM)
>>>
>>> /* CTRLMMR_ICSSG_RGMII_CTRL register bits */
>>> #define ICSSG_CTRL_RGMII_ID_MODE BIT(24)
>>> @@ -758,6 +761,21 @@ static void emac_change_hsr_feature(struct net_device *ndev,
>>> }
>>> }
>>>
>>> +static netdev_features_t emac_ndo_fix_features(struct net_device *ndev,
>>> + netdev_features_t features)
>>> +{
>>> + /* In order to enable hsr tag insertion offload, hsr dup offload must
>>> + * also be enabled as these two are tightly coupled in firmware
>>> + * implementation.
>>> + */
>>> + if (features & NETIF_F_HW_HSR_TAG_INS)
>>> + features |= NETIF_F_HW_HSR_DUP;
>>
>> What if only NETIF_F_HW_HSR_DUP was set? Don't you have to set NETIF_F_HW_HSR_TAG_INS as well?
>>
>>> + else
>>> + features &= ~NETIF_F_HW_HSR_DUP;
>>
>> what if NETIF_F_HW_HSR_DUP was still set?
>>
>> I think you need to write a logic like follows.
>> if both are already cleared in ndev->features and any one is set in features you set both in features.
>> if both are already set in ndev->features and any one is cleared in features you clear both in features.
>>
>> is this reasonable?
>>
>
> Yes that does sound reasonable,
> How does below code look to you.
>
> if (!(ndev->features & NETIF_F_HW_HSR_DUP) &&
> !(ndev->features & NETIF_F_HW_HSR_TAG_INS))
While this is OK. it could also be
if (!(ndev->features & (NETIF_F_HW_HSR_DUP | NETIF_F_HW_HSR_TAG_INS)))
Your choice.
> if ((features & NETIF_F_HW_HSR_DUP) ||
> (features & NETIF_F_HW_HSR_TAG_INS)) {
> features |= NETIF_F_HW_HSR_DUP;
> features |= NETIF_F_HW_HSR_TAG_INS;
how about one line?
features |= NETIF_F_HW_HSR_DUP | NETIF_F_HW_HSR_TAG_INS;
> }
then you can get rid of the braces.
>
> if ((ndev->features & NETIF_F_HW_HSR_DUP) &&
> (ndev->features & NETIF_F_HW_HSR_TAG_INS))
> if (!(features & NETIF_F_HW_HSR_DUP) ||
> !(features & NETIF_F_HW_HSR_TAG_INS)) {
> features &= ~NETIF_F_HW_HSR_DUP;
> features &= ~NETIF_F_HW_HSR_TAG_INS;
features &= ~(NETIF_F_HW_HSR_DUP | NETIF_F_HW_HSR_TAG_INS);
> }
>
>>> +
>>> + return features;
>>> +}
>>> +
>>> static int emac_ndo_set_features(struct net_device *ndev,
>>> netdev_features_t features)
>>> {
>>> @@ -780,6 +798,7 @@ static const struct net_device_ops emac_netdev_ops = {
>>> .ndo_eth_ioctl = icssg_ndo_ioctl,
>>> .ndo_get_stats64 = icssg_ndo_get_stats64,
>>> .ndo_get_phys_port_name = icssg_ndo_get_phys_port_name,
>>> + .ndo_fix_features = emac_ndo_fix_features,
>>> .ndo_set_features = emac_ndo_set_features,
>>> };
>>>
>>> @@ -1007,6 +1026,13 @@ static void icssg_change_mode(struct prueth *prueth)
>>>
>>> for (mac = PRUETH_MAC0; mac < PRUETH_NUM_MACS; mac++) {
>>> emac = prueth->emac[mac];
>>> + if (prueth->is_hsr_offload_mode) {
>>> + if (emac->ndev->features & NETIF_F_HW_HSR_TAG_RM)
>>> + icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE);
>>> + else
>>> + icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE);
>>> + }
>>> +
>>> if (netif_running(emac->ndev)) {
>>> icssg_fdb_add_del(emac, eth_stp_addr, prueth->default_vlan,
>>> ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>> index a4b025fae797..bba6da2e6bd8 100644
>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>> @@ -59,6 +59,9 @@
>>>
>>> #define IEP_DEFAULT_CYCLE_TIME_NS 1000000 /* 1 ms */
>>>
>>> +#define PRUETH_UNDIRECTED_PKT_DST_TAG 0
>>> +#define PRUETH_UNDIRECTED_PKT_TAG_INS BIT(30)
>>> +
>>> /* Firmware status codes */
>>> #define ICSS_HS_FW_READY 0x55555555
>>> #define ICSS_HS_FW_DEAD 0xDEAD0000 /* lower 16 bits contain error code */
>>
>
--
cheers,
-roger
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v5 4/5] net: ti: icssg-prueth: Enable HSR Tx duplication, Tx Tag and Rx Tag offload
2024-09-10 18:06 ` Roger Quadros
@ 2024-09-10 18:27 ` Anwar, Md Danish
2024-09-10 19:15 ` Roger Quadros
0 siblings, 1 reply; 13+ messages in thread
From: Anwar, Md Danish @ 2024-09-10 18:27 UTC (permalink / raw)
To: Roger Quadros, MD Danish Anwar, robh, jan.kiszka, dan.carpenter,
saikrishnag, andrew, javier.carrasco.cruz, jacob.e.keller,
diogo.ivo, horms, richardcochran, pabeni, kuba, edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
Ravi Gunasekaran
On 9/10/2024 11:36 PM, Roger Quadros wrote:
>
>
> On 10/09/2024 20:52, Anwar, Md Danish wrote:
>> Hi Roger,
>>
>> On 9/10/2024 11:08 PM, Roger Quadros wrote:
>>>
>>>
>>> On 06/09/2024 14:15, MD Danish Anwar wrote:
>>>> From: Ravi Gunasekaran <r-gunasekaran@ti.com>
>>>>
>>>> The HSR stack allows to offload its Tx packet duplication functionality to
>>>> the hardware. Enable this offloading feature for ICSSG driver. Add support
>>>> to offload HSR Tx Tag Insertion and Rx Tag Removal and duplicate discard.
>>>>
>>>> Inorder to enable hsr-tag-ins-offload, hsr-dup-offload must also be enabled
>>>
>>> "In order"
>>>
>>>> as these are tightly coupled in the firmware implementation.
>>>>
>>>> Duplicate discard is done as part of RX tag removal and it is
>>>> done by the firmware. When driver sends the r30 command
>>>> ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE, firmware does RX tag removal as well as
>>>> duplicate discard.
>>>>
>>>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>> ---
>>>> drivers/net/ethernet/ti/icssg/icssg_common.c | 18 ++++++++++---
>>>> drivers/net/ethernet/ti/icssg/icssg_config.c | 4 ++-
>>>> drivers/net/ethernet/ti/icssg/icssg_config.h | 2 ++
>>>> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 28 +++++++++++++++++++-
>>>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 3 +++
>>>> 5 files changed, 50 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
>>>> index b9d8a93d1680..fdebeb2f84e0 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
>>>> @@ -660,14 +660,15 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
>>>> {
>>>> struct cppi5_host_desc_t *first_desc, *next_desc, *cur_desc;
>>>> struct prueth_emac *emac = netdev_priv(ndev);
>>>> + struct prueth *prueth = emac->prueth;
>>>> struct netdev_queue *netif_txq;
>>>> struct prueth_tx_chn *tx_chn;
>>>> dma_addr_t desc_dma, buf_dma;
>>>> + u32 pkt_len, dst_tag_id;
>>>> int i, ret = 0, q_idx;
>>>> bool in_tx_ts = 0;
>>>> int tx_ts_cookie;
>>>> void **swdata;
>>>> - u32 pkt_len;
>>>> u32 *epib;
>>>>
>>>> pkt_len = skb_headlen(skb);
>>>> @@ -712,9 +713,20 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
>>>>
>>>> /* set dst tag to indicate internal qid at the firmware which is at
>>>> * bit8..bit15. bit0..bit7 indicates port num for directed
>>>> - * packets in case of switch mode operation
>>>> + * packets in case of switch mode operation and port num 0
>>>> + * for undirected packets in case of HSR offload mode
>>>> */
>>>> - cppi5_desc_set_tags_ids(&first_desc->hdr, 0, (emac->port_id | (q_idx << 8)));
>>>> + dst_tag_id = emac->port_id | (q_idx << 8);
>>>> +
>>>> + if (prueth->is_hsr_offload_mode &&
>>>> + (ndev->features & NETIF_F_HW_HSR_DUP))
>>>> + dst_tag_id = PRUETH_UNDIRECTED_PKT_DST_TAG;
>>>> +
>>>> + if (prueth->is_hsr_offload_mode &&
>>>> + (ndev->features & NETIF_F_HW_HSR_TAG_INS))
>>>> + epib[1] |= PRUETH_UNDIRECTED_PKT_TAG_INS;
>>>> +
>>>> + cppi5_desc_set_tags_ids(&first_desc->hdr, 0, dst_tag_id);
>>>> k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
>>>> cppi5_hdesc_attach_buf(first_desc, buf_dma, pkt_len, buf_dma, pkt_len);
>>>> swdata = cppi5_hdesc_get_swdata(first_desc);
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>>> index 7b2e6c192ff3..72ace151d8e9 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>>> @@ -531,7 +531,9 @@ static const struct icssg_r30_cmd emac_r32_bitmask[] = {
>>>> {{EMAC_NONE, 0xffff4000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx ENABLE*/
>>>> {{EMAC_NONE, 0xbfff0000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx DISABLE*/
>>>> {{0xffff0010, EMAC_NONE, 0xffff0010, EMAC_NONE}}, /* VLAN AWARE*/
>>>> - {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}} /* VLAN UNWARE*/
>>>> + {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}}, /* VLAN UNWARE*/
>>>> + {{0xffff2000, EMAC_NONE, EMAC_NONE, EMAC_NONE}}, /* HSR_RX_OFFLOAD_ENABLE */
>>>> + {{0xdfff0000, EMAC_NONE, EMAC_NONE, EMAC_NONE}} /* HSR_RX_OFFLOAD_DISABLE */
>>>> };
>>>>
>>>> int icssg_set_port_state(struct prueth_emac *emac,
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
>>>> index 1ac60283923b..92c2deaa3068 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
>>>> @@ -80,6 +80,8 @@ enum icssg_port_state_cmd {
>>>> ICSSG_EMAC_PORT_PREMPT_TX_DISABLE,
>>>> ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE,
>>>> ICSSG_EMAC_PORT_VLAN_AWARE_DISABLE,
>>>> + ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE,
>>>> + ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE,
>>>> ICSSG_EMAC_PORT_MAX_COMMANDS
>>>> };
>>>>
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>>>> index 676168d6fded..9af06454ba64 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>>>> @@ -41,7 +41,10 @@
>>>> #define DEFAULT_PORT_MASK 1
>>>> #define DEFAULT_UNTAG_MASK 1
>>>>
>>>> -#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES NETIF_F_HW_HSR_FWD
>>>> +#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES (NETIF_F_HW_HSR_FWD | \
>>>> + NETIF_F_HW_HSR_DUP | \
>>>> + NETIF_F_HW_HSR_TAG_INS | \
>>>> + NETIF_F_HW_HSR_TAG_RM)
>>>>
>>>> /* CTRLMMR_ICSSG_RGMII_CTRL register bits */
>>>> #define ICSSG_CTRL_RGMII_ID_MODE BIT(24)
>>>> @@ -758,6 +761,21 @@ static void emac_change_hsr_feature(struct net_device *ndev,
>>>> }
>>>> }
>>>>
>>>> +static netdev_features_t emac_ndo_fix_features(struct net_device *ndev,
>>>> + netdev_features_t features)
>>>> +{
>>>> + /* In order to enable hsr tag insertion offload, hsr dup offload must
>>>> + * also be enabled as these two are tightly coupled in firmware
>>>> + * implementation.
>>>> + */
>>>> + if (features & NETIF_F_HW_HSR_TAG_INS)
>>>> + features |= NETIF_F_HW_HSR_DUP;
>>>
>>> What if only NETIF_F_HW_HSR_DUP was set? Don't you have to set NETIF_F_HW_HSR_TAG_INS as well?
>>>
>>>> + else
>>>> + features &= ~NETIF_F_HW_HSR_DUP;
>>>
>>> what if NETIF_F_HW_HSR_DUP was still set?
>>>
>>> I think you need to write a logic like follows.
>>> if both are already cleared in ndev->features and any one is set in features you set both in features.
>>> if both are already set in ndev->features and any one is cleared in features you clear both in features.
>>>
>>> is this reasonable?
>>>
>>
>> Yes that does sound reasonable,
>> How does below code look to you.
>>
>> if (!(ndev->features & NETIF_F_HW_HSR_DUP) &&
>> !(ndev->features & NETIF_F_HW_HSR_TAG_INS))
>
> While this is OK. it could also be
> if (!(ndev->features & (NETIF_F_HW_HSR_DUP | NETIF_F_HW_HSR_TAG_INS)))
> Your choice.
>
>> if ((features & NETIF_F_HW_HSR_DUP) ||
>> (features & NETIF_F_HW_HSR_TAG_INS)) {
>> features |= NETIF_F_HW_HSR_DUP;
>> features |= NETIF_F_HW_HSR_TAG_INS;
> how about one line?
> features |= NETIF_F_HW_HSR_DUP | NETIF_F_HW_HSR_TAG_INS;
>> }
>
> then you can get rid of the braces.
>
Yes I could do this but this make the line length go beyond 80
characters. That's why I thought of putting this in two lines.
>>
>> if ((ndev->features & NETIF_F_HW_HSR_DUP) &&
>> (ndev->features & NETIF_F_HW_HSR_TAG_INS))
>> if (!(features & NETIF_F_HW_HSR_DUP) ||
>> !(features & NETIF_F_HW_HSR_TAG_INS)) {
>> features &= ~NETIF_F_HW_HSR_DUP;
>> features &= ~NETIF_F_HW_HSR_TAG_INS;
>
> features &= ~(NETIF_F_HW_HSR_DUP | NETIF_F_HW_HSR_TAG_INS);
>
>> }
>>
>>>> +
>>>> + return features;
>>>> +}
>>>> +
>>>> static int emac_ndo_set_features(struct net_device *ndev,
>>>> netdev_features_t features)
>>>> {
>>>> @@ -780,6 +798,7 @@ static const struct net_device_ops emac_netdev_ops = {
>>>> .ndo_eth_ioctl = icssg_ndo_ioctl,
>>>> .ndo_get_stats64 = icssg_ndo_get_stats64,
>>>> .ndo_get_phys_port_name = icssg_ndo_get_phys_port_name,
>>>> + .ndo_fix_features = emac_ndo_fix_features,
>>>> .ndo_set_features = emac_ndo_set_features,
>>>> };
>>>>
>>>> @@ -1007,6 +1026,13 @@ static void icssg_change_mode(struct prueth *prueth)
>>>>
>>>> for (mac = PRUETH_MAC0; mac < PRUETH_NUM_MACS; mac++) {
>>>> emac = prueth->emac[mac];
>>>> + if (prueth->is_hsr_offload_mode) {
>>>> + if (emac->ndev->features & NETIF_F_HW_HSR_TAG_RM)
>>>> + icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE);
>>>> + else
>>>> + icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE);
>>>> + }
>>>> +
>>>> if (netif_running(emac->ndev)) {
>>>> icssg_fdb_add_del(emac, eth_stp_addr, prueth->default_vlan,
>>>> ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> index a4b025fae797..bba6da2e6bd8 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> @@ -59,6 +59,9 @@
>>>>
>>>> #define IEP_DEFAULT_CYCLE_TIME_NS 1000000 /* 1 ms */
>>>>
>>>> +#define PRUETH_UNDIRECTED_PKT_DST_TAG 0
>>>> +#define PRUETH_UNDIRECTED_PKT_TAG_INS BIT(30)
>>>> +
>>>> /* Firmware status codes */
>>>> #define ICSS_HS_FW_READY 0x55555555
>>>> #define ICSS_HS_FW_DEAD 0xDEAD0000 /* lower 16 bits contain error code */
>>>
>>
>
--
Thanks and Regards,
Md Danish Anwar
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v5 4/5] net: ti: icssg-prueth: Enable HSR Tx duplication, Tx Tag and Rx Tag offload
2024-09-10 18:27 ` Anwar, Md Danish
@ 2024-09-10 19:15 ` Roger Quadros
0 siblings, 0 replies; 13+ messages in thread
From: Roger Quadros @ 2024-09-10 19:15 UTC (permalink / raw)
To: Anwar, Md Danish, MD Danish Anwar, robh, jan.kiszka,
dan.carpenter, saikrishnag, andrew, javier.carrasco.cruz,
jacob.e.keller, diogo.ivo, horms, richardcochran, pabeni, kuba,
edumazet, davem
Cc: linux-kernel, netdev, linux-arm-kernel, srk, Vignesh Raghavendra,
Ravi Gunasekaran
On 10/09/2024 21:27, Anwar, Md Danish wrote:
>
>
> On 9/10/2024 11:36 PM, Roger Quadros wrote:
>>
>>
>> On 10/09/2024 20:52, Anwar, Md Danish wrote:
>>> Hi Roger,
>>>
>>> On 9/10/2024 11:08 PM, Roger Quadros wrote:
>>>>
>>>>
>>>> On 06/09/2024 14:15, MD Danish Anwar wrote:
>>>>> From: Ravi Gunasekaran <r-gunasekaran@ti.com>
>>>>>
>>>>> The HSR stack allows to offload its Tx packet duplication functionality to
>>>>> the hardware. Enable this offloading feature for ICSSG driver. Add support
>>>>> to offload HSR Tx Tag Insertion and Rx Tag Removal and duplicate discard.
>>>>>
>>>>> Inorder to enable hsr-tag-ins-offload, hsr-dup-offload must also be enabled
>>>>
>>>> "In order"
>>>>
>>>>> as these are tightly coupled in the firmware implementation.
>>>>>
>>>>> Duplicate discard is done as part of RX tag removal and it is
>>>>> done by the firmware. When driver sends the r30 command
>>>>> ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE, firmware does RX tag removal as well as
>>>>> duplicate discard.
>>>>>
>>>>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
>>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>>> ---
>>>>> drivers/net/ethernet/ti/icssg/icssg_common.c | 18 ++++++++++---
>>>>> drivers/net/ethernet/ti/icssg/icssg_config.c | 4 ++-
>>>>> drivers/net/ethernet/ti/icssg/icssg_config.h | 2 ++
>>>>> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 28 +++++++++++++++++++-
>>>>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 3 +++
>>>>> 5 files changed, 50 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
>>>>> index b9d8a93d1680..fdebeb2f84e0 100644
>>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
>>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
>>>>> @@ -660,14 +660,15 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
>>>>> {
>>>>> struct cppi5_host_desc_t *first_desc, *next_desc, *cur_desc;
>>>>> struct prueth_emac *emac = netdev_priv(ndev);
>>>>> + struct prueth *prueth = emac->prueth;
>>>>> struct netdev_queue *netif_txq;
>>>>> struct prueth_tx_chn *tx_chn;
>>>>> dma_addr_t desc_dma, buf_dma;
>>>>> + u32 pkt_len, dst_tag_id;
>>>>> int i, ret = 0, q_idx;
>>>>> bool in_tx_ts = 0;
>>>>> int tx_ts_cookie;
>>>>> void **swdata;
>>>>> - u32 pkt_len;
>>>>> u32 *epib;
>>>>>
>>>>> pkt_len = skb_headlen(skb);
>>>>> @@ -712,9 +713,20 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
>>>>>
>>>>> /* set dst tag to indicate internal qid at the firmware which is at
>>>>> * bit8..bit15. bit0..bit7 indicates port num for directed
>>>>> - * packets in case of switch mode operation
>>>>> + * packets in case of switch mode operation and port num 0
>>>>> + * for undirected packets in case of HSR offload mode
>>>>> */
>>>>> - cppi5_desc_set_tags_ids(&first_desc->hdr, 0, (emac->port_id | (q_idx << 8)));
>>>>> + dst_tag_id = emac->port_id | (q_idx << 8);
>>>>> +
>>>>> + if (prueth->is_hsr_offload_mode &&
>>>>> + (ndev->features & NETIF_F_HW_HSR_DUP))
>>>>> + dst_tag_id = PRUETH_UNDIRECTED_PKT_DST_TAG;
>>>>> +
>>>>> + if (prueth->is_hsr_offload_mode &&
>>>>> + (ndev->features & NETIF_F_HW_HSR_TAG_INS))
>>>>> + epib[1] |= PRUETH_UNDIRECTED_PKT_TAG_INS;
>>>>> +
>>>>> + cppi5_desc_set_tags_ids(&first_desc->hdr, 0, dst_tag_id);
>>>>> k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
>>>>> cppi5_hdesc_attach_buf(first_desc, buf_dma, pkt_len, buf_dma, pkt_len);
>>>>> swdata = cppi5_hdesc_get_swdata(first_desc);
>>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>>>> index 7b2e6c192ff3..72ace151d8e9 100644
>>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
>>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>>>> @@ -531,7 +531,9 @@ static const struct icssg_r30_cmd emac_r32_bitmask[] = {
>>>>> {{EMAC_NONE, 0xffff4000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx ENABLE*/
>>>>> {{EMAC_NONE, 0xbfff0000, EMAC_NONE, EMAC_NONE}}, /* Preemption on Tx DISABLE*/
>>>>> {{0xffff0010, EMAC_NONE, 0xffff0010, EMAC_NONE}}, /* VLAN AWARE*/
>>>>> - {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}} /* VLAN UNWARE*/
>>>>> + {{0xffef0000, EMAC_NONE, 0xffef0000, EMAC_NONE}}, /* VLAN UNWARE*/
>>>>> + {{0xffff2000, EMAC_NONE, EMAC_NONE, EMAC_NONE}}, /* HSR_RX_OFFLOAD_ENABLE */
>>>>> + {{0xdfff0000, EMAC_NONE, EMAC_NONE, EMAC_NONE}} /* HSR_RX_OFFLOAD_DISABLE */
>>>>> };
>>>>>
>>>>> int icssg_set_port_state(struct prueth_emac *emac,
>>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
>>>>> index 1ac60283923b..92c2deaa3068 100644
>>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
>>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
>>>>> @@ -80,6 +80,8 @@ enum icssg_port_state_cmd {
>>>>> ICSSG_EMAC_PORT_PREMPT_TX_DISABLE,
>>>>> ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE,
>>>>> ICSSG_EMAC_PORT_VLAN_AWARE_DISABLE,
>>>>> + ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE,
>>>>> + ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE,
>>>>> ICSSG_EMAC_PORT_MAX_COMMANDS
>>>>> };
>>>>>
>>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>>>>> index 676168d6fded..9af06454ba64 100644
>>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
>>>>> @@ -41,7 +41,10 @@
>>>>> #define DEFAULT_PORT_MASK 1
>>>>> #define DEFAULT_UNTAG_MASK 1
>>>>>
>>>>> -#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES NETIF_F_HW_HSR_FWD
>>>>> +#define NETIF_PRUETH_HSR_OFFLOAD_FEATURES (NETIF_F_HW_HSR_FWD | \
>>>>> + NETIF_F_HW_HSR_DUP | \
>>>>> + NETIF_F_HW_HSR_TAG_INS | \
>>>>> + NETIF_F_HW_HSR_TAG_RM)
>>>>>
>>>>> /* CTRLMMR_ICSSG_RGMII_CTRL register bits */
>>>>> #define ICSSG_CTRL_RGMII_ID_MODE BIT(24)
>>>>> @@ -758,6 +761,21 @@ static void emac_change_hsr_feature(struct net_device *ndev,
>>>>> }
>>>>> }
>>>>>
>>>>> +static netdev_features_t emac_ndo_fix_features(struct net_device *ndev,
>>>>> + netdev_features_t features)
>>>>> +{
>>>>> + /* In order to enable hsr tag insertion offload, hsr dup offload must
>>>>> + * also be enabled as these two are tightly coupled in firmware
>>>>> + * implementation.
>>>>> + */
>>>>> + if (features & NETIF_F_HW_HSR_TAG_INS)
>>>>> + features |= NETIF_F_HW_HSR_DUP;
>>>>
>>>> What if only NETIF_F_HW_HSR_DUP was set? Don't you have to set NETIF_F_HW_HSR_TAG_INS as well?
>>>>
>>>>> + else
>>>>> + features &= ~NETIF_F_HW_HSR_DUP;
>>>>
>>>> what if NETIF_F_HW_HSR_DUP was still set?
>>>>
>>>> I think you need to write a logic like follows.
>>>> if both are already cleared in ndev->features and any one is set in features you set both in features.
>>>> if both are already set in ndev->features and any one is cleared in features you clear both in features.
>>>>
>>>> is this reasonable?
>>>>
>>>
>>> Yes that does sound reasonable,
>>> How does below code look to you.
>>>
>>> if (!(ndev->features & NETIF_F_HW_HSR_DUP) &&
>>> !(ndev->features & NETIF_F_HW_HSR_TAG_INS))
>>
>> While this is OK. it could also be
>> if (!(ndev->features & (NETIF_F_HW_HSR_DUP | NETIF_F_HW_HSR_TAG_INS)))
>> Your choice.
>>
>>> if ((features & NETIF_F_HW_HSR_DUP) ||
>>> (features & NETIF_F_HW_HSR_TAG_INS)) {
>>> features |= NETIF_F_HW_HSR_DUP;
>>> features |= NETIF_F_HW_HSR_TAG_INS;
>> how about one line?
>> features |= NETIF_F_HW_HSR_DUP | NETIF_F_HW_HSR_TAG_INS;
>>> }
>>
>> then you can get rid of the braces.
>>
>
> Yes I could do this but this make the line length go beyond 80
> characters. That's why I thought of putting this in two lines.
then you can split the line
e.g.
features |= NETIF_F_HW_HSR_DUP |
NETIF_F_HW_HSR_TAG_INS;
>
>>>
>>> if ((ndev->features & NETIF_F_HW_HSR_DUP) &&
>>> (ndev->features & NETIF_F_HW_HSR_TAG_INS))
>>> if (!(features & NETIF_F_HW_HSR_DUP) ||
>>> !(features & NETIF_F_HW_HSR_TAG_INS)) {
>>> features &= ~NETIF_F_HW_HSR_DUP;
>>> features &= ~NETIF_F_HW_HSR_TAG_INS;
>>
>> features &= ~(NETIF_F_HW_HSR_DUP | NETIF_F_HW_HSR_TAG_INS);
>>
>>> }
>>>
>>>>> +
>>>>> + return features;
>>>>> +}
>>>>> +
>>>>> static int emac_ndo_set_features(struct net_device *ndev,
>>>>> netdev_features_t features)
>>>>> {
>>>>> @@ -780,6 +798,7 @@ static const struct net_device_ops emac_netdev_ops = {
>>>>> .ndo_eth_ioctl = icssg_ndo_ioctl,
>>>>> .ndo_get_stats64 = icssg_ndo_get_stats64,
>>>>> .ndo_get_phys_port_name = icssg_ndo_get_phys_port_name,
>>>>> + .ndo_fix_features = emac_ndo_fix_features,
>>>>> .ndo_set_features = emac_ndo_set_features,
>>>>> };
>>>>>
>>>>> @@ -1007,6 +1026,13 @@ static void icssg_change_mode(struct prueth *prueth)
>>>>>
>>>>> for (mac = PRUETH_MAC0; mac < PRUETH_NUM_MACS; mac++) {
>>>>> emac = prueth->emac[mac];
>>>>> + if (prueth->is_hsr_offload_mode) {
>>>>> + if (emac->ndev->features & NETIF_F_HW_HSR_TAG_RM)
>>>>> + icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE);
>>>>> + else
>>>>> + icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE);
>>>>> + }
>>>>> +
>>>>> if (netif_running(emac->ndev)) {
>>>>> icssg_fdb_add_del(emac, eth_stp_addr, prueth->default_vlan,
>>>>> ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
>>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>>> index a4b025fae797..bba6da2e6bd8 100644
>>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>>> @@ -59,6 +59,9 @@
>>>>>
>>>>> #define IEP_DEFAULT_CYCLE_TIME_NS 1000000 /* 1 ms */
>>>>>
>>>>> +#define PRUETH_UNDIRECTED_PKT_DST_TAG 0
>>>>> +#define PRUETH_UNDIRECTED_PKT_TAG_INS BIT(30)
>>>>> +
>>>>> /* Firmware status codes */
>>>>> #define ICSS_HS_FW_READY 0x55555555
>>>>> #define ICSS_HS_FW_DEAD 0xDEAD0000 /* lower 16 bits contain error code */
>>>>
>>>
>>
>
--
cheers,
-roger
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-09-10 19:15 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-06 11:15 [PATCH net-next v5 0/5] Introduce HSR offload support for ICSSG MD Danish Anwar
2024-09-06 11:15 ` [PATCH net-next v5 1/5] net: ti: icss-iep: Move icss_iep structure MD Danish Anwar
2024-09-06 11:15 ` [PATCH net-next v5 2/5] net: ti: icssg-prueth: Stop hardcoding def_inc MD Danish Anwar
2024-09-06 11:15 ` [PATCH net-next v5 3/5] net: ti: icssg-prueth: Add support for HSR frame forward offload MD Danish Anwar
2024-09-10 12:12 ` Roger Quadros
2024-09-10 13:13 ` Anwar, Md Danish
2024-09-06 11:15 ` [PATCH net-next v5 4/5] net: ti: icssg-prueth: Enable HSR Tx duplication, Tx Tag and Rx Tag offload MD Danish Anwar
2024-09-10 17:38 ` Roger Quadros
2024-09-10 17:52 ` Anwar, Md Danish
2024-09-10 18:06 ` Roger Quadros
2024-09-10 18:27 ` Anwar, Md Danish
2024-09-10 19:15 ` Roger Quadros
2024-09-06 11:15 ` [PATCH net-next v5 5/5] net: ti: icssg-prueth: Add multicast filtering support in HSR mode MD Danish Anwar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).