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 11/12/2023 14:11, Vladimir Oltean wrote:
> On Fri, Dec 08, 2023 at 12:26:24PM +0200, Roger Quadros wrote:
>> On 08/12/2023 12:13, Roger Quadros wrote:
>>> 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
> 
> I don't know, does it sound like it is related?
> 
> config TI_AM65_CPSW_TAS
> 	bool "Enable TAS offload in AM65 CPSW"
> 	depends on TI_K3_AM65_CPSW_NUSS && NET_SCH_TAPRIO && TI_K3_AM65_CPTS
> 	help
> 	  Say y here to support Time Aware Shaper(TAS) offload in AM65 CPSW.
> 	  AM65 CPSW hardware supports Enhanced Scheduled Traffic (EST)
> 	  defined in IEEE 802.1Q 2018. The EST scheduler runs on CPTS and the
> 	  TAS/EST schedule is updated in the Fetch RAM memory of the CPSW.

The config option mentions only about TAS/EST 802.1Qbv.
Maybe it needs to be extended to include IET/Frame-preeption 802.1Qbu as well?

This is the simplest way as the file am65-cpsw-qos.c can be completely omitted if
TI_AM65_CPSW_TSN is not set.

-- 
cheers,
-roger