Re: [PATCH v10 2/2] net: ti: icssg-prueth: Add ICSSG ethernet driver

[Jakub Kicinski <kuba@kernel.org>]

The patch is too big to review.

Please break it apart separating into individual features, targeting
around 10 patches in the series. That will make it easier for reviewers
to take a look at the features in which they have expertise.

See two things which jumped out at me immediately below:

On Wed, 19 Jul 2023 13:57:55 +0530 MD Danish Anwar wrote:
> +	ICSSG_STATS(rx_crc_error_frames),

> +	ICSSG_STATS(rx_max_size_error_frames),
> +	ICSSG_STATS(rx_frame_min_size),
> +	ICSSG_STATS(rx_min_size_error_frames),
> +	ICSSG_STATS(rx_overrun_frames),

> +	ICSSG_STATS(rx_64B_frames),
> +	ICSSG_STATS(rx_bucket1_frames),
> +	ICSSG_STATS(rx_bucket2_frames),
> +	ICSSG_STATS(rx_bucket3_frames),
> +	ICSSG_STATS(rx_bucket4_frames),
> +	ICSSG_STATS(rx_bucket5_frames),
> +	ICSSG_STATS(rx_total_bytes),
> +	ICSSG_STATS(rx_tx_total_bytes),
> +	/* Tx */
> +	ICSSG_STATS(tx_good_frames),
> +	ICSSG_STATS(tx_broadcast_frames),
> +	ICSSG_STATS(tx_multicast_frames),
> +	ICSSG_STATS(tx_odd_nibble_frames),
> +	ICSSG_STATS(tx_underflow_errors),
> +	ICSSG_STATS(tx_frame_max_size),
> +	ICSSG_STATS(tx_max_size_error_frames),
> +	ICSSG_STATS(tx_frame_min_size),
> +	ICSSG_STATS(tx_min_size_error_frames),
> +	ICSSG_STATS(tx_bucket1_size),
> +	ICSSG_STATS(tx_bucket2_size),
> +	ICSSG_STATS(tx_bucket3_size),
> +	ICSSG_STATS(tx_bucket4_size),
> +	ICSSG_STATS(tx_64B_frames),
> +	ICSSG_STATS(tx_bucket1_frames),
> +	ICSSG_STATS(tx_bucket2_frames),
> +	ICSSG_STATS(tx_bucket3_frames),
> +	ICSSG_STATS(tx_bucket4_frames),
> +	ICSSG_STATS(tx_bucket5_frames),
> +	ICSSG_STATS(tx_total_bytes),

Please use standard stats:
https://docs.kernel.org/next/networking/statistics.html

And do not duplicate those stats in the ethool -S output.

> +static const char emac_ethtool_priv_flags[][ETH_GSTRING_LEN] = {
> +	"iet-frame-preemption",
> +	"iet-mac-verify",
> +};

What are these? We have a proper ethtool API for frame preemption.
-- 
pw-bot: cr