Re: [PATCH v2] net: ethernet: ti: am65-cpsw: add mqprio qdisc offload in channel mode
flat view
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: 2023-09-19 12:47:46
Also in:
lkml
Hi Roger, On Mon, Sep 18, 2023 at 10:53:58AM +0300, Roger Quadros wrote:
-int am65_cpsw_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
- void *type_data)
-{
- switch (type) {
- case TC_QUERY_CAPS:
- return am65_cpsw_tc_query_caps(ndev, type_data);
- case TC_SETUP_QDISC_TAPRIO:
- return am65_cpsw_setup_taprio(ndev, type_data);
- case TC_SETUP_BLOCK:
- return am65_cpsw_qos_setup_tc_block(ndev, type_data);
- default:
- return -EOPNOTSUPP;
- }
-}
-
-void am65_cpsw_qos_link_up(struct net_device *ndev, int link_speed)
-{
- struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
-
- if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS))
- return;
-
- am65_cpsw_est_link_up(ndev, link_speed);
- port->qos.link_down_time = 0;
-}
-
-void am65_cpsw_qos_link_down(struct net_device *ndev)
-{
- struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
-
- if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS))
- return;
-
- if (!port->qos.link_down_time)
- port->qos.link_down_time = ktime_get();
-
- port->qos.link_speed = SPEED_UNKNOWN;
-}
-Could you split the code movement to a separate change?
quoted hunk
+ if (port->qos.link_speed != SPEED_UNKNOWN) { + if (min_rate_total > port->qos.link_speed) { + NL_SET_ERR_MSG_FMT_MOD(extack, "TX rate min %llu exceeds link speed %d\n", + min_rate_total, port->qos.link_speed); + return -EINVAL; + } + + if (max_rate_total > port->qos.link_speed) { + NL_SET_ERR_MSG_FMT_MOD(extack, "TX rate max %llu exceeds link speed %d\n", + max_rate_total, port->qos.link_speed); + return -EINVAL; + } + }
Link speeds can be renegotiated, and the mqprio offload can be installed while the link is down. So this restriction, while honorable, has limited usefulness.
+ + p_mqprio->shaper_en = 1;
s/1/true/
quoted hunk
+ p_mqprio->max_rate_total = max_t(u64, min_rate_total, max_rate_total); + + return 0; +} + +static void am65_cpsw_reset_tc_mqprio(struct net_device *ndev) +{ + struct am65_cpsw_port *port = am65_ndev_to_port(ndev); + struct am65_cpsw_mqprio *p_mqprio = &port->qos.mqprio; + struct am65_cpsw_common *common = port->common; + + p_mqprio->shaper_en = 0;
s/0/false/
quoted hunk
+ p_mqprio->max_rate_total = 0; + + am65_cpsw_tx_pn_shaper_reset(port); + netdev_reset_tc(ndev); + netif_set_real_num_tx_queues(ndev, common->tx_ch_num); + + /* Reset all Queue priorities to 0 */ + writel(0, + port->port_base + AM65_CPSW_PN_REG_TX_PRI_MAP);
What exactly needs pm_runtime_get_sync()? This writel() doesn't?
quoted hunk
+} + +static int am65_cpsw_setup_mqprio(struct net_device *ndev, void *type_data) +{ + struct am65_cpsw_port *port = am65_ndev_to_port(ndev); + struct am65_cpsw_mqprio *p_mqprio = &port->qos.mqprio; + struct tc_mqprio_qopt_offload *mqprio = type_data; + struct am65_cpsw_common *common = port->common; + struct tc_mqprio_qopt *qopt = &mqprio->qopt; + int tc, offset, count, ret, prio; + u8 num_tc = qopt->num_tc; + u32 tx_prio_map = 0; + int i; + + memcpy(&p_mqprio->mqprio_hw, mqprio, sizeof(*mqprio)); + + if (!num_tc) { + am65_cpsw_reset_tc_mqprio(ndev); + return 0; + } + + ret = pm_runtime_get_sync(common->dev); + if (ret < 0) { + pm_runtime_put_noidle(common->dev); + return ret; + }