Re: [PATCH net-next v5 05/11] net: ti: prueth: Adds ethtool support for ICSSM PRUETH Driver

[Simon Horman <horms@kernel.org>]

On Wed, Apr 23, 2025 at 05:28:16PM +0100, Simon Horman wrote:
> On Mon, Apr 14, 2025 at 06:32:31PM +0530, Parvathi Pudi wrote:
> > From: Roger Quadros <rogerq@ti.com>
> > 
> > Changes for enabling ethtool support for the newly added PRU Ethernet
> > interfaces. Extends the support for statistics collection from PRU internal
> > memory and displays it in the user space. Along with statistics,
> > enable/disable of features, configuring link speed etc.are now supported.
> > 
> > The firmware running on PRU maintains statistics in internal data memory.
> > When requested ethtool collects all the statistics for the specified
> > interface and displays it in the user space.
> > 
> > Makefile is updated to include ethtool support into PRUETH driver.
> > 
> > Signed-off-by: Roger Quadros <rogerq@ti.com>
> > Signed-off-by: Andrew F. Davis <afd@ti.com>
> > Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com>
> > Signed-off-by: Parvathi Pudi <parvathi@couthit.com>
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.h b/drivers/net/ethernet/ti/icssm/icssm_prueth.h
> 
> ...
> 
> > @@ -108,6 +114,119 @@ struct prueth_packet_info {
> >  	bool timestamp;
> >  };
> >  
> > +/**
> > + * struct port_statistics - Statistics structure for capturing statistics
> > + *			    on PRUs
> > + * @tx_bcast: Number of broadcast packets sent
> > + * @tx_mcast:Number of multicast packets sent
> > + * @tx_ucast:Number of unicast packets sent
> > + *
> > + * @tx_octets:Number of undersized frames rcvd
> > + *
> > + * @rx_bcast:Number of broadcast packets rcvd
> > + * @rx_mcast:Number of multicast packets rcvd
> > + * @rx_ucast:Number of unicast packets rcvd
> > + *
> > + * @rx_octets:Number of Rx packets
> > + *
> > + * @tx64byte:Number of 64 byte packets sent
> > + * @tx65_127byte:Number of 65-127 byte packets sent
> > + * @tx128_255byte:Number of 128-255 byte packets sent
> > + * @tx256_511byte:Number of 256-511 byte packets sent
> > + * @tx512_1023byte:Number of 512-1023 byte packets sent
> > + * @tx1024byte:Number of 1024 and larger size packets sent
> > + *
> > + * @rx64byte:Number of 64 byte packets rcvd
> > + * @rx65_127byte:Number of 65-127 byte packets rcvd
> > + * @rx128_255byte:Number of 128-255 byte packets rcvd
> > + * @rx256_511byte:Number of 256-511 byte packets rcvd
> > + * @rx512_1023byte:Number of 512-1023 byte packets rcvd
> > + * @rx1024byte:Number of 1024 and larger size packets rcvd
> > + *
> > + * @late_coll:Number of late collisions(Half Duplex)
> > + * @single_coll:Number of single collisions (Half Duplex)
> > + * @multi_coll:Number of multiple collisions (Half Duplex)
> > + * @excess_coll:Number of excess collisions(Half Duplex)
> > + *
> > + * @rx_misalignment_frames:Number of non multiple of 8 byte frames rcvd
> > + * @stormprev_counter:Number of packets dropped because of Storm Prevention
> 
> nit: It looks like the documentation of @stormprev_counter should
>      be replaced by documentation of:
>      @u32 stormprev_counter_bc;
>      @u32 stormprev_counter_mc;
>      @u32 stormprev_counter_uc;
> 
> > + * @mac_rxerror:Number of MAC receive errors
> > + * @sfd_error:Number of invalid SFD
> > + * @def_tx:Number of transmissions deferred
> > + * @mac_txerror:Number of MAC transmit errors
> > + * @rx_oversized_frames:Number of oversized frames rcvd
> > + * @rx_undersized_frames:Number of undersized frames rcvd
> > + * @rx_crc_frames:Number of CRC error frames rcvd
> > + * @dropped_packets:Number of packets dropped due to link down on opposite port
> > + *
> > + * @tx_hwq_overflow:Hardware Tx Queue (on PRU) over flow count
> > + * @tx_hwq_underflow:Hardware Tx Queue (on PRU) under flow count
> > + *
> > + * @u32 cs_error: Number of carrier sense errors
> 
> nit: @cs_error
> 
>      i.e. remove "u32 "
> 
> Documentation nits flagged by ./scripts/kernel-doc -none
> 
> > + * @sqe_test_error: Number of MAC receive errors
> > + *
> > + * Above fields are aligned so that it's consistent
> > + * with the memory layout in PRU DRAM, this is to facilitate easy
> > + * memcpy. Don't change the order of the fields.
> > + *
> > + * @vlan_dropped: Number of VLAN tagged packets dropped
> > + * @multicast_dropped: Number of multicast packets dropped
> > + */
> > +struct port_statistics {
> > +	u32 tx_bcast;
> > +	u32 tx_mcast;
> > +	u32 tx_ucast;
> > +
> > +	u32 tx_octets;
> > +
> > +	u32 rx_bcast;
> > +	u32 rx_mcast;
> > +	u32 rx_ucast;
> > +
> > +	u32 rx_octets;
> > +
> > +	u32 tx64byte;
> > +	u32 tx65_127byte;
> > +	u32 tx128_255byte;
> > +	u32 tx256_511byte;
> > +	u32 tx512_1023byte;
> > +	u32 tx1024byte;
> > +
> > +	u32 rx64byte;
> > +	u32 rx65_127byte;
> > +	u32 rx128_255byte;
> > +	u32 rx256_511byte;
> > +	u32 rx512_1023byte;
> > +	u32 rx1024byte;
> > +
> > +	u32 late_coll;
> > +	u32 single_coll;
> > +	u32 multi_coll;
> > +	u32 excess_coll;
> > +
> > +	u32 rx_misalignment_frames;
> > +	u32 stormprev_counter_bc;
> > +	u32 stormprev_counter_mc;
> > +	u32 stormprev_counter_uc;
> > +	u32 mac_rxerror;
> > +	u32 sfd_error;
> > +	u32 def_tx;
> > +	u32 mac_txerror;
> > +	u32 rx_oversized_frames;
> > +	u32 rx_undersized_frames;
> > +	u32 rx_crc_frames;
> > +	u32 dropped_packets;
> > +
> > +	u32 tx_hwq_overflow;
> > +	u32 tx_hwq_underflow;
> > +
> > +	u32 cs_error;
> > +	u32 sqe_test_error;
> > +
> > +	u32 vlan_dropped;
> > +	u32 multicast_dropped;
> > +} __packed;
> 
> ...

Oops, I seem to have replied to an old version of this patch-set.
My comments above do still seem relevant to v6.
But as v6 exits the comment below is nonsense. Sorry about that.

> 
> The above notwithstanding, it seems based on comment's elsewhere in this
> thread that there will be another revision of this patchset. Let patchwork
> know about that.
> 
> pw-bot: changes-requested
>