Messages in this thread | | | Date | Fri, 22 Sep 2023 13:40:21 +0300 | Subject | Re: [PATCH net-next v2] net: ti: icssg_prueth: add TAPRIO offload support | From | Roger Quadros <> |
| |
On 22/09/2023 07:58, MD Danish Anwar wrote: > On 21/09/23 16:12, Roger Quadros wrote: >> Hi Danish, >> >> On 21/09/2023 10:00, MD Danish Anwar wrote: >> >> Can you please retain patch authorhsip? >> > > Sure Roger. This patch was not applying clearly on latest linux-next so > I had to manually apply it. I must have lost authorship while doing > this. I will reset the authorship. > >>> ICSSG dual-emac f/w supports Enhanced Scheduled Traffic (EST – defined >>> in P802.1Qbv/D2.2 that later got included in IEEE 802.1Q-2018) >>> configuration. EST allows express queue traffic to be scheduled >>> (placed) on the wire at specific repeatable time intervals. In >>> Linux kernel, EST configuration is done through tc command and >>> the taprio scheduler in the net core implements a software only >>> scheduler (SCH_TAPRIO). If the NIC is capable of EST configuration, >>> user indicate "flag 2" in the command which is then parsed by >>> taprio scheduler in net core and indicate that the command is to >>> be offloaded to h/w. taprio then offloads the command to the >>> driver by calling ndo_setup_tc() ndo ops. This patch implements >>> ndo_setup_tc() to offload EST configuration to ICSSG. >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com> >>> --- >>> Cc: Roger Quadros <rogerq@ti.com> >>> Cc: Andrew Lunn <andrew@lunn.ch> >>> >>> Changes from v1 to v2: >>> *) Rebased on the latest next-20230821 linux-next. >>> *) Dropped the RFC tag as merge window is open now. >>> *) Splitted this patch from the switch mode series [v1]. >>> *) Removed TODO comment as asked by Andrew and Roger. >>> *) Changed Copyright to 2023 as asked by Roger. >>> >>> v1: https://lore.kernel.org/all/20230830110847.1219515-1-danishanwar@ti.com/ >>> >>> drivers/net/ethernet/ti/Makefile | 3 +- >>> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 5 +- >>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 7 + >>> drivers/net/ethernet/ti/icssg/icssg_qos.c | 286 +++++++++++++++++++ >>> drivers/net/ethernet/ti/icssg/icssg_qos.h | 119 ++++++++ >>> 5 files changed, 418 insertions(+), 2 deletions(-) >>> create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.c >>> create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.h >>> >>> diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile >>> index 34fd7a716ba6..0df60ded1b2d 100644 >>> --- a/drivers/net/ethernet/ti/Makefile >>> +++ b/drivers/net/ethernet/ti/Makefile >>> @@ -37,5 +37,6 @@ icssg-prueth-y := k3-cppi-desc-pool.o \ >>> icssg/icssg_config.o \ >>> icssg/icssg_mii_cfg.o \ >>> icssg/icssg_stats.o \ >>> - icssg/icssg_ethtool.o >>> + icssg/icssg_ethtool.o \ >>> + icssg/icssg_qos.o >>> obj-$(CONFIG_TI_ICSS_IEP) += icssg/icss_iep.o >>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c >>> index 6635b28bc672..89c301716926 100644 >>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c >>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c >>> @@ -1166,7 +1166,7 @@ static int emac_phy_connect(struct prueth_emac *emac) >>> return 0; >>> } >>> >>> -static u64 prueth_iep_gettime(void *clockops_data, struct ptp_system_timestamp *sts) >>> +u64 prueth_iep_gettime(void *clockops_data, struct ptp_system_timestamp *sts) >>> { >>> u32 hi_rollover_count, hi_rollover_count_r; >>> struct prueth_emac *emac = clockops_data; >>> @@ -1403,6 +1403,8 @@ static int emac_ndo_open(struct net_device *ndev) >>> napi_enable(&emac->tx_chns[i].napi_tx); >>> napi_enable(&emac->napi_rx); >>> >>> + icssg_qos_tas_init(ndev); >>> + >>> /* start PHY */ >>> phy_start(ndev->phydev); >>> >>> @@ -1669,6 +1671,7 @@ static const struct net_device_ops emac_netdev_ops = { >>> .ndo_set_rx_mode = emac_ndo_set_rx_mode, >>> .ndo_eth_ioctl = emac_ndo_ioctl, >>> .ndo_get_stats64 = emac_ndo_get_stats64, >>> + .ndo_setup_tc = icssg_qos_ndo_setup_tc, >>> }; >>> >>> /* get emac_port corresponding to eth_node name */ >>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h >>> index 8b6d6b497010..5712a65bced4 100644 >>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h >>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h >>> @@ -37,6 +37,7 @@ >>> #include "icssg_config.h" >>> #include "icss_iep.h" >>> #include "icssg_switch_map.h" >>> +#include "icssg_qos.h" >>> >>> #define PRUETH_MAX_MTU (2000 - ETH_HLEN - ETH_FCS_LEN) >>> #define PRUETH_MIN_PKT_SIZE (VLAN_ETH_ZLEN) >>> @@ -174,6 +175,9 @@ struct prueth_emac { >>> >>> struct pruss_mem_region dram; >>> >>> + struct prueth_qos qos; >>> + struct work_struct ts_work; >>> + >>> struct delayed_work stats_work; >>> u64 stats[ICSSG_NUM_STATS]; >>> }; >>> @@ -285,4 +289,7 @@ u32 icssg_queue_level(struct prueth *prueth, int queue); >>> void emac_stats_work_handler(struct work_struct *work); >>> void emac_update_hardware_stats(struct prueth_emac *emac); >>> int emac_get_stat_by_name(struct prueth_emac *emac, char *stat_name); >>> + >>> +u64 prueth_iep_gettime(void *clockops_data, struct ptp_system_timestamp *sts); >>> + >>> #endif /* __NET_TI_ICSSG_PRUETH_H */ >>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.c b/drivers/net/ethernet/ti/icssg/icssg_qos.c >>> new file mode 100644 >>> index 000000000000..63a19142ee69 >>> --- /dev/null >>> +++ b/drivers/net/ethernet/ti/icssg/icssg_qos.c >>> @@ -0,0 +1,286 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* Texas Instruments ICSSG PRUETH QoS submodule >>> + * Copyright (C) 2023 Texas Instruments Incorporated - http://www.ti.com/ >>> + */ >>> + >>> +#include <linux/printk.h> >>> +#include "icssg_prueth.h" >>> +#include "icssg_switch_map.h" >>> + >>> +static void tas_update_fw_list_pointers(struct prueth_emac *emac) >>> +{ >>> + struct tas_config *tas = &emac->qos.tas.config; >>> + >>> + if ((readb(tas->active_list)) == TAS_LIST0) { >>> + tas->fw_active_list = emac->dram.va + TAS_GATE_MASK_LIST0; >>> + tas->fw_shadow_list = emac->dram.va + TAS_GATE_MASK_LIST1; >>> + } else { >>> + tas->fw_active_list = emac->dram.va + TAS_GATE_MASK_LIST1; >>> + tas->fw_shadow_list = emac->dram.va + TAS_GATE_MASK_LIST0; >>> + } >>> +} >>> + >>> +static void tas_update_maxsdu_table(struct prueth_emac *emac) >>> +{ >>> + struct tas_config *tas = &emac->qos.tas.config; >>> + u16 __iomem *max_sdu_tbl_ptr; >>> + u8 gate_idx; >>> + >>> + /* update the maxsdu table */ >>> + max_sdu_tbl_ptr = emac->dram.va + TAS_QUEUE_MAX_SDU_LIST; >>> + >>> + for (gate_idx = 0; gate_idx < TAS_MAX_NUM_QUEUES; gate_idx++) >>> + writew(tas->max_sdu_table.max_sdu[gate_idx], &max_sdu_tbl_ptr[gate_idx]); >>> +} >>> + >>> +static void tas_reset(struct prueth_emac *emac) >>> +{ >>> + struct tas_config *tas = &emac->qos.tas.config; >>> + int i; >>> + >>> + for (i = 0; i < TAS_MAX_NUM_QUEUES; i++) >>> + tas->max_sdu_table.max_sdu[i] = 2048; >>> + >>> + tas_update_maxsdu_table(emac); >>> + >>> + writeb(TAS_LIST0, tas->active_list); >>> + >>> + memset_io(tas->fw_active_list, 0, sizeof(*tas->fw_active_list)); >>> + memset_io(tas->fw_shadow_list, 0, sizeof(*tas->fw_shadow_list)); >>> +} >>> + >>> +static int tas_set_state(struct prueth_emac *emac, enum tas_state state) >>> +{ >>> + struct tas_config *tas = &emac->qos.tas.config; >>> + int ret; >>> + >>> + if (tas->state == state) >>> + return 0; >>> + >>> + switch (state) { >>> + case TAS_STATE_RESET: >>> + tas_reset(emac); >>> + ret = emac_set_port_state(emac, ICSSG_EMAC_PORT_TAS_RESET); >>> + tas->state = TAS_STATE_RESET; >>> + break; >>> + case TAS_STATE_ENABLE: >>> + ret = emac_set_port_state(emac, ICSSG_EMAC_PORT_TAS_ENABLE); >>> + tas->state = TAS_STATE_ENABLE; >>> + break; >>> + case TAS_STATE_DISABLE: >>> + ret = emac_set_port_state(emac, ICSSG_EMAC_PORT_TAS_DISABLE); >>> + tas->state = TAS_STATE_DISABLE; >>> + break; >>> + default: >>> + netdev_err(emac->ndev, "%s: unsupported state\n", __func__); >>> + ret = -EINVAL; >>> + break; >>> + } >>> + >>> + if (ret) >>> + netdev_err(emac->ndev, "TAS set state failed %d\n", ret); >>> + return ret; >>> +} >>> + >>> +static int tas_set_trigger_list_change(struct prueth_emac *emac) >>> +{ >>> + struct tc_taprio_qopt_offload *admin_list = emac->qos.tas.taprio_admin; >>> + struct tas_config *tas = &emac->qos.tas.config; >>> + struct ptp_system_timestamp sts; >>> + u32 change_cycle_count; >>> + u32 cycle_time; >>> + u64 base_time; >>> + u64 cur_time; >>> + >>> + cycle_time = admin_list->cycle_time - 4; /* -4ns to compensate for IEP wraparound time */ >>> + base_time = admin_list->base_time; >>> + cur_time = prueth_iep_gettime(emac, &sts); >>> + >>> + if (base_time > cur_time) >>> + change_cycle_count = DIV_ROUND_UP_ULL(base_time - cur_time, cycle_time); >>> + else >>> + change_cycle_count = 1; >>> + >>> + writel(cycle_time, emac->dram.va + TAS_ADMIN_CYCLE_TIME); >>> + writel(change_cycle_count, emac->dram.va + TAS_CONFIG_CHANGE_CYCLE_COUNT); >>> + writeb(admin_list->num_entries, emac->dram.va + TAS_ADMIN_LIST_LENGTH); >>> + >>> + /* config_change cleared by f/w to ack reception of new shadow list */ >>> + writeb(1, &tas->config_list->config_change); >>> + /* config_pending cleared by f/w when new shadow list is copied to active list */ >>> + writeb(1, &tas->config_list->config_pending); >>> + >>> + return emac_set_port_state(emac, ICSSG_EMAC_PORT_TAS_TRIGGER); >>> +} >>> + >>> +static int tas_update_oper_list(struct prueth_emac *emac) >>> +{ >>> + struct tc_taprio_qopt_offload *admin_list = emac->qos.tas.taprio_admin; >>> + struct tas_config *tas = &emac->qos.tas.config; >>> + u32 tas_acc_gate_close_time = 0; >>> + u8 idx, gate_idx, val; >>> + int ret; >>> + >>> + tas_update_fw_list_pointers(emac); >>> + >>> + for (idx = 0; idx < admin_list->num_entries; idx++) { >>> + writeb(admin_list->entries[idx].gate_mask, >>> + &tas->fw_shadow_list->gate_mask_list[idx]); >>> + tas_acc_gate_close_time += admin_list->entries[idx].interval; >>> + >>> + /* extend last entry till end of cycle time */ >>> + if (idx == admin_list->num_entries - 1) >>> + writel(admin_list->cycle_time, >>> + &tas->fw_shadow_list->win_end_time_list[idx]); >>> + else >>> + writel(tas_acc_gate_close_time, >>> + &tas->fw_shadow_list->win_end_time_list[idx]); >>> + } >>> + >>> + /* clear remaining entries */ >>> + for (idx = admin_list->num_entries; idx < TAS_MAX_CMD_LISTS; idx++) { >>> + writeb(0, &tas->fw_shadow_list->gate_mask_list[idx]); >>> + writel(0, &tas->fw_shadow_list->win_end_time_list[idx]); >>> + } >>> + >>> + /* update the Array of gate close time for each queue in each window */ >>> + for (idx = 0 ; idx < admin_list->num_entries; idx++) { >>> + /* On Linux, only PRUETH_MAX_TX_QUEUES are supported per port */ >>> + for (gate_idx = 0; gate_idx < PRUETH_MAX_TX_QUEUES; gate_idx++) { >>> + u8 gate_mask_list_idx = readb(&tas->fw_shadow_list->gate_mask_list[idx]); >>> + u32 gate_close_time = 0; >>> + >>> + if (gate_mask_list_idx & BIT(gate_idx)) >>> + gate_close_time = readl(&tas->fw_shadow_list->win_end_time_list[idx]); >>> + >>> + writel(gate_close_time, >>> + &tas->fw_shadow_list->gate_close_time_list[idx][gate_idx]); >>> + } >>> + } >>> + >>> + /* tell f/w to swap active & shadow list */ >>> + ret = tas_set_trigger_list_change(emac); >>> + if (ret) { >>> + netdev_err(emac->ndev, "failed to swap f/w config list: %d\n", ret); >>> + return ret; >>> + } >>> + >>> + /* Wait for completion */ >>> + ret = readb_poll_timeout(&tas->config_list->config_change, val, !val, >>> + USEC_PER_MSEC, 10 * USEC_PER_MSEC); >>> + if (ret) { >>> + netdev_err(emac->ndev, "TAS list change completion time out\n"); >>> + return ret; >>> + } >>> + >>> + tas_update_fw_list_pointers(emac); >>> + >>> + return 0; >>> +} >>> + >>> +static int emac_set_taprio(struct prueth_emac *emac) >>> +{ >>> + struct tc_taprio_qopt_offload *taprio = emac->qos.tas.taprio_admin; >>> + int ret; >>> + >>> + if (taprio->cmd == TAPRIO_CMD_DESTROY) >>> + return tas_set_state(emac, TAS_STATE_DISABLE); >>> + >>> + if (taprio->cmd != TAPRIO_CMD_REPLACE) >>> + return -EOPNOTSUPP; >>> + >>> + ret = tas_update_oper_list(emac); >>> + if (ret) >>> + return ret; >>> + >>> + return tas_set_state(emac, TAS_STATE_ENABLE); >>> +} >>> + >>> +static void emac_cp_taprio(struct tc_taprio_qopt_offload *from, >>> + struct tc_taprio_qopt_offload *to) >>> +{ >>> + int i; >>> + >>> + *to = *from; >>> + for (i = 0; i < from->num_entries; i++) >>> + to->entries[i] = from->entries[i]; >>> +} >>> + >>> +static int emac_setup_taprio(struct net_device *ndev, struct tc_taprio_qopt_offload *taprio) >> >> Please change this to >> static int emac_setup_taprio(struct net_device *ndev, void *type_data) >> >> and later add >> struct tc_taprio_qopt_offload *taprio = type_data; > > Sure. > >>> +{ >>> + struct prueth_emac *emac = netdev_priv(ndev); >>> + struct tc_taprio_qopt_offload *est_new; >>> + int ret, idx; >>> + >>> + if (!netif_running(ndev)) { >>> + netdev_err(ndev, "interface is down, link speed unknown\n"); >>> + return -ENETDOWN; >>> + } >> >> Do we really need this? >> >> How about handling the taprio->cmd with a switch statement >> and adding helper functions for each case? >> > > emac_set_taprio() is already doing something like this. > It only implements TAPRIO_CMD_REPLACE and TAPRIO_CMD_DESTROY. Others are > not supported. > > static int emac_set_taprio(struct prueth_emac *emac) > { > struct tc_taprio_qopt_offload *taprio = emac->qos.tas.taprio_admin; > int ret; > > if (taprio->cmd == TAPRIO_CMD_DESTROY) > return tas_set_state(emac, TAS_STATE_DISABLE); > > if (taprio->cmd != TAPRIO_CMD_REPLACE) > return -EOPNOTSUPP; > > ret = tas_update_oper_list(emac); > if (ret) > return ret; > > return tas_set_state(emac, TAS_STATE_ENABLE); > } > > emac_setup_taprio() is first doing all the neccessary check and then > calling emac_set_taprio(), which actually perform actions based on the > taprio->cmd. I think I'll keep this as it is.
OK but pleae at least use switch statement in emac_set_taprio() and error out in default: case.
> >> So emac_setup_taprio() reduces to >> >> static int emac_setup_taprio(struct net_device *ndev, void *type_data) >> { >> struct tc_taprio_qopt_offload *taprio = type_data; >> int err = 0; >> >> switch (taprio->cmd) { >> case TAPRIO_CMD_REPLACE: >> err = emac_taprio_replace(ndev, taprio); >> break; >> case TAPRIO_CMD_DESTROY: >> emac_taprio_destroy(ndev); >> break; >> case TAPRIO_CMD_STATS: >> emac_taprio_stats(ndev, &taprio->stats); >> break; >> case TAPRIO_CMD_QUEUE_STATS: >> emac_taprio_queue_stats(ndev, &taprio->queue_stats); >> break; >> default: >> err = -EOPNOTSUPP; >> } >> >> return err; >> } >>
-- cheers, -roger
| |