Re: [PATCH v7 net-next 6/8] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support
From: Roger Quadros <rogerq@kernel.org>
Date: 2023-12-08 12:33:06
On 08/12/2023 12:26, Roger Quadros wrote:
quoted hunk ↗ jump to hunk
On 08/12/2023 12:13, Roger Quadros wrote:quoted
On 04/12/2023 14:35, Vladimir Oltean wrote:quoted
On Fri, Dec 01, 2023 at 03:58:00PM +0200, Roger Quadros wrote:quoted
Add driver support for viewing / changing the MAC Merge sublayer parameters and seeing the verification state machine's current state via ethtool. As hardware does not support interrupt notification for verification events we resort to polling on link up. On link up we try a couple of times for verification success and if unsuccessful then give up. The Frame Preemption feature is described in the Technical Reference Manual [1] in section: 12.3.1.4.6.7 Intersperced Express Traffic (IET – P802.3br/D2.0) Due to Silicon Errata i2208 [2] we set limit min IET fragment size to 124. [1] AM62x TRM - https://www.ti.com/lit/ug/spruiv7a/spruiv7a.pdf [2] AM62x Silicon Errata - https://www.ti.com/lit/er/sprz487c/sprz487c.pdf Signed-off-by: Roger Quadros <rogerq@kernel.org> ---Actually... ld.lld: error: undefined symbol: am65_cpsw_iet_common_enablequoted
quoted
quoted
referenced by am65-cpsw-ethtool.c:755 (drivers/net/ethernet/ti/am65-cpsw-ethtool.c:755) drivers/net/ethernet/ti/am65-cpsw-ethtool.o:(am65_cpsw_set_mm) in archive vmlinux.ald.lld: error: undefined symbol: am65_cpsw_iet_commit_preemptible_tcsquoted
quoted
quoted
referenced by am65-cpsw-ethtool.c:876 (drivers/net/ethernet/ti/am65-cpsw-ethtool.c:876) drivers/net/ethernet/ti/am65-cpsw-ethtool.o:(am65_cpsw_set_mm) in archive vmlinux.acat $KBUILD_OUTPUT/.config | grep AM65 CONFIG_TI_K3_AM65_CPSW_NUSS=y # CONFIG_TI_K3_AM65_CPSW_SWITCHDEV is not set # CONFIG_TI_K3_AM65_CPTS is not set CONFIG_MMC_SDHCI_AM654=y CONFIG_PHY_AM654_SERDES=m am65-cpsw-qos.c is built only if CONFIG_TI_AM65_CPSW_TAS is enabled, yet am65-cpsw-ethtool.c, built by CONFIG_TI_K3_AM65_CPSW_NUSS, depends on it.Wondering how to fix this the right way. Should set/get_mm fail if CONFIG_TI_AM65_CPSW_TAS is not enabled?How about this fix?diff --git a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c index 1ac4b9b53c93..688291d6038f 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c +++ b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c@@ -775,6 +775,9 @@ static int am65_cpsw_get_mm(struct net_device *ndev, struct ethtool_mm_state *st u32 port_ctrl, iet_ctrl, iet_status; u32 add_frag_size; + if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS)) + return -EOPNOTSUPP; + mutex_lock(&priv->mm_lock); iet_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);@@ -827,6 +830,9 @@ static int am65_cpsw_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg, u32 val, add_frag_size; int err; + if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS)) + return -EOPNOTSUPP; + err = ethtool_mm_frag_size_min_to_add(cfg->tx_min_frag_size, &add_frag_size, extack); if (err) return err;diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.h b/drivers/net/ethernet/ti/am65-cpsw-qos.h index 6df3c2c5a04b..946e89fbb314 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-qos.h +++ b/drivers/net/ethernet/ti/am65-cpsw-qos.h@@ -100,6 +100,8 @@ void am65_cpsw_qos_link_up(struct net_device *ndev, int link_speed); void am65_cpsw_qos_link_down(struct net_device *ndev); int am65_cpsw_qos_ndo_tx_p0_set_maxrate(struct net_device *ndev, int queue, u32 rate_mbps); void am65_cpsw_qos_tx_p0_rate_init(struct am65_cpsw_common *common); +void am65_cpsw_iet_commit_preemptible_tcs(struct am65_cpsw_port *port); +void am65_cpsw_iet_common_enable(struct am65_cpsw_common *common); #else static inline int am65_cpsw_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,@@ -124,10 +126,12 @@ static inline int am65_cpsw_qos_ndo_tx_p0_set_maxrate(struct net_device *ndev, static inline void am65_cpsw_qos_tx_p0_rate_init(struct am65_cpsw_common *common) { } +static inline void am65_cpsw_iet_commit_preemptible_tcs(struct am65_cpsw_port *port) +{ } +static inline void am65_cpsw_iet_common_enable(struct am65_cpsw_common *common) +{ } #endif -void am65_cpsw_iet_commit_preemptible_tcs(struct am65_cpsw_port *port); -void am65_cpsw_iet_common_enable(struct am65_cpsw_common *common); #define AM65_CPSW_REG_CTL 0x004 #define AM65_CPSW_PN_REG_CTL 0x004
But,
bool __ethtool_dev_mm_supported(struct net_device *dev)
{
const struct ethtool_ops *ops = dev->ethtool_ops;
struct ethtool_mm_state state = {};
int ret = -EOPNOTSUPP;
if (ops && ops->get_mm)
ret = ops->get_mm(dev, &state);
return !ret;
}
So looks like it is better to not define get_mm/set_mm if CONFIG_TI_AM65_CPSW_TAS is disabled.
--
cheers,
-roger