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 15:29, Vladimir Oltean wrote:
> On Fri, Dec 08, 2023 at 03:43:45PM +0200, Roger Quadros wrote:
>> How do the below 2 patches look to resolve this?
>>
>> From af2a8503dc04c54d6eaf50954628009aba54e2c8 Mon Sep 17 00:00:00 2001
>> From: Roger Quadros <rogerq@kernel.org>
>> Date: Fri, 8 Dec 2023 15:11:06 +0200
>> Subject: [PATCH] net: ethernet: ti: am65-cpsw: Fix get_eth_mac_stats
>>
>> We do not support individual stats for PMAC and EMAC so
>> report only aggregate stats.
>>
>> Fixes: 67372d7a85fcd ("net: ethernet: am65-cpsw: Add standard Ethernet MAC stats to ethtool")
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 37 ++++++++++++---------
>>  1 file changed, 22 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
>> index 44e01d68db39..934b03382508 100644
>> --- a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
>> +++ b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
>> @@ -671,21 +671,28 @@ static void am65_cpsw_get_eth_mac_stats(struct net_device *ndev,
>>  
>>  	stats = port->stat_base;
>>  
>> -	s->FramesTransmittedOK = readl_relaxed(&stats->tx_good_frames);
>> -	s->SingleCollisionFrames = readl_relaxed(&stats->tx_single_coll_frames);
>> -	s->MultipleCollisionFrames = readl_relaxed(&stats->tx_mult_coll_frames);
>> -	s->FramesReceivedOK = readl_relaxed(&stats->rx_good_frames);
>> -	s->FrameCheckSequenceErrors = readl_relaxed(&stats->rx_crc_errors);
>> -	s->AlignmentErrors = readl_relaxed(&stats->rx_align_code_errors);
>> -	s->OctetsTransmittedOK = readl_relaxed(&stats->tx_octets);
>> -	s->FramesWithDeferredXmissions = readl_relaxed(&stats->tx_deferred_frames);
>> -	s->LateCollisions = readl_relaxed(&stats->tx_late_collisions);
>> -	s->CarrierSenseErrors = readl_relaxed(&stats->tx_carrier_sense_errors);
>> -	s->OctetsReceivedOK = readl_relaxed(&stats->rx_octets);
>> -	s->MulticastFramesXmittedOK = readl_relaxed(&stats->tx_multicast_frames);
>> -	s->BroadcastFramesXmittedOK = readl_relaxed(&stats->tx_broadcast_frames);
>> -	s->MulticastFramesReceivedOK = readl_relaxed(&stats->rx_multicast_frames);
>> -	s->BroadcastFramesReceivedOK = readl_relaxed(&stats->rx_broadcast_frames);
>> +        switch (s->src) {
>> +	case ETHTOOL_MAC_STATS_SRC_AGGREGATE:
>> +		s->FramesTransmittedOK = readl_relaxed(&stats->tx_good_frames);
>> +		s->SingleCollisionFrames = readl_relaxed(&stats->tx_single_coll_frames);
>> +		s->MultipleCollisionFrames = readl_relaxed(&stats->tx_mult_coll_frames);
>> +		s->FramesReceivedOK = readl_relaxed(&stats->rx_good_frames);
>> +		s->FrameCheckSequenceErrors = readl_relaxed(&stats->rx_crc_errors);
>> +		s->AlignmentErrors = readl_relaxed(&stats->rx_align_code_errors);
>> +		s->OctetsTransmittedOK = readl_relaxed(&stats->tx_octets);
>> +		s->FramesWithDeferredXmissions = readl_relaxed(&stats->tx_deferred_frames);
>> +		s->LateCollisions = readl_relaxed(&stats->tx_late_collisions);
>> +		s->CarrierSenseErrors = readl_relaxed(&stats->tx_carrier_sense_errors);
>> +		s->OctetsReceivedOK = readl_relaxed(&stats->rx_octets);
>> +		s->MulticastFramesXmittedOK = readl_relaxed(&stats->tx_multicast_frames);
>> +		s->BroadcastFramesXmittedOK = readl_relaxed(&stats->tx_broadcast_frames);
>> +		s->MulticastFramesReceivedOK = readl_relaxed(&stats->rx_multicast_frames);
>> +		s->BroadcastFramesReceivedOK = readl_relaxed(&stats->rx_broadcast_frames);
>> +		break;
>> +	default:
>> +		break;
>> +	}
>>  };
> 
> 	if (s->src != ETHTOOL_MAC_STATS_SRC_AGGREGATE)
> 		return;

Is better.

> 
> Also, again, tabs mixed with spaces. What editor are you using, notepad?

I'm using vim but I didn't run checkpatch on it before pasting
it here for quick feedback.

> 
>>  
>>  static int am65_cpsw_get_ethtool_ts_info(struct net_device *ndev,
>>
>> base-commit: a78e0a2c4353d6c100e45c5ef738113bf2d0fda5
>> -- 
>> 2.34.1
>>
>>
>>
>>
>> From 0b20d8b8ef110d886396ee2486f3a9e20170cc85 Mon Sep 17 00:00:00 2001
>> From: Roger Quadros <rogerq@kernel.org>
>> Date: Fri, 8 Dec 2023 15:38:57 +0200
>> Subject: [PATCH] selftests: forwarding: ethtool_mm: support devices that don't
>>  support pmac stats
>>
>> Some devices do not support individual 'pmac' and 'emac' stats.
>> For such devices, resort to 'aggregate' stats.
>>
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  tools/testing/selftests/net/forwarding/ethtool_mm.sh | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
>> index 6212913f4ad1..e3f2e62029ca 100755
>> --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
>> +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
>> @@ -26,6 +26,13 @@ traffic_test()
>>  	local delta=
>>  
>>  	before=$(ethtool_std_stats_get $if "eth-mac" "FramesTransmittedOK" $src)
>> +	# some devices don't support individual pmac/emac stats,
>> +	# use aggregate stats for them.
>> +        if [ "$before" == null ]; then
>> +                src="aggregate"
>> +                before=$(ethtool_std_stats_get $if "eth-mac" "FramesTransmittedOO
>> +K" $src)
>> +        fi
>>  
>>  	$MZ $if -q -c $num_pkts -p 64 -b bcast -t ip -R $PREEMPTIBLE_PRIO
>>  
>>
>> base-commit: af2a8503dc04c54d6eaf50954628009aba54e2c8
>> -- 
>> 2.34.1
>>
>> -- 
>> cheers,
>> -roger
> 
> I commented on the second patch when you submitted it.

-- 
cheers,
-roger