Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 19 Jul 2023 21:35:43 -0700
From:   Jakub Kicinski <kuba@kernel.org>
To:     MD Danish Anwar <danishanwar@ti.com>
Cc:     Randy Dunlap <rdunlap@infradead.org>,
        Roger Quadros <rogerq@kernel.org>,
        Simon Horman <simon.horman@corigine.com>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Richard Cochran <richardcochran@gmail.com>,
        Conor Dooley <conor+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Eric Dumazet <edumazet@google.com>,
        "David S. Miller" <davem@davemloft.net>, <nm@ti.com>, <srk@ti.com>,
        <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <netdev@vger.kernel.org>, <linux-omap@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v10 2/2] net: ti: icssg-prueth: Add ICSSG ethernet
 driver
Message-ID: <20230719213543.0380e13e@kernel.org>
In-Reply-To: <20230719082755.3399424-3-danishanwar@ti.com>
References: <20230719082755.3399424-1-danishanwar@ti.com>
        <20230719082755.3399424-3-danishanwar@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

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