Re: [PATCH v7 net-next 6/8] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support

[Roger Quadros <rogerq@kernel.org>]

On 08/12/2023 12:13, Roger Quadros wrote:
> 
> 
> On 04/12/2023 14:35, Vladimir Oltean wrote:
>> On Fri, Dec 01, 2023 at 03:58:00PM +0200, Roger Quadros wrote:
>>> 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_enable
>>>>> 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.a
>>
>> ld.lld: error: undefined symbol: am65_cpsw_iet_commit_preemptible_tcs
>>>>> 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.a
>>
>> cat $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


-- 
cheers,
-roger