[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7d8fb848-a491-414b-adb8-d26a16a499a4@kernel.org>
Date: Fri, 8 Dec 2023 12:26:24 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, s-vadapalli@...com, r-gunasekaran@...com,
vigneshr@...com, srk@...com, horms@...nel.org, p-varis@...com,
netdev@...r.kernel.org
Subject: Re: [PATCH v7 net-next 6/8] net: ethernet: ti: am65-cpsw-qos: Add
Frame Preemption MAC Merge support
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@...nel.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
Powered by blists - more mailing lists