Re: [RFC v2 PATCH 04/10] net: ti: prueth: Adds link detection, RX and TX support.

Basharath Hussain Khaja
2025-02-01T13:37:27Z

> On Fri, Jan 24, 2025 at 06:07:01PM +0530, Basharath Hussain Khaja wrote:
>> From: Roger Quadros <rogerq@ti.com>
>>
>> Changes corresponding to link configuration such as speed and duplexity.
>> IRQ and handler initializations are performed for packet reception.Firmware
>> receives the packet from the wire and stores it into OCMC queue. Next, it
>> notifies the CPU via interrupt. Upon receiving the interrupt CPU will
>> service the IRQ and packet will be processed by pushing the newly allocated
>> SKB to upper layers.
>>
>> When the user application want to transmit a packet, it will invoke
>> sys_send() which will inturn invoke the PRUETH driver, then it will write
>> the packet into OCMC queues. PRU firmware will pick up the packet and
>> transmit it on to the wire.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>> Signed-off-by: Parvathi Pudi <parvathi@couthit.com>
>> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com>
>> ---
>> drivers/net/ethernet/ti/icssm/icssm_prueth.c | 599 ++++++++++++++++++-
>> drivers/net/ethernet/ti/icssm/icssm_prueth.h | 46 ++
>> 2 files changed, 640 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.c
>> b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
>
> ...
>
>> +/**
>> + * icssm_emac_ndo_start_xmit - EMAC Transmit function
>> + * @skb: SKB pointer
>> + * @ndev: EMAC network adapter
>> + *
>> + * Called by the system to transmit a packet - we queue the packet in
>> + * EMAC hardware transmit queue
>> + *
>> + * Return: success(NETDEV_TX_OK) or error code (typically out of desc's)
>> + */
>> +static int icssm_emac_ndo_start_xmit(struct sk_buff *skb,
>> + struct net_device *ndev)
>
> I think the return type of this function should be netdev_tx_t
> rather than int to match the signature of ndo_start_xmit
> in struct net_device_ops.
>
> Flagged by W=1 build with clang-19
> (-Wincompatible-function-pointer-types-strict).
>

We will change the return type in the next version.

> ...
>
>> static const struct net_device_ops emac_netdev_ops = {
>> .ndo_open = icssm_emac_ndo_open,
>> .ndo_stop = icssm_emac_ndo_stop,
>> + .ndo_start_xmit = icssm_emac_ndo_start_xmit,
>> };
>>
>> /* get emac_port corresponding to eth_node name */
>
> ...
>
>> diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.h
>> b/drivers/net/ethernet/ti/icssm/icssm_prueth.h
>
> ...
>
>> @@ -76,6 +82,32 @@ struct prueth_queue_info {
>> u16 buffer_desc_end;
>> } __packed;
>>
>> +/**
>> + * struct prueth_packet_info - Info about a packet in buffer
>> + * @start_offset: start offset of the frame in the buffer for HSR/PRP
>> + * @shadow: this packet is stored in the collision queue
>> + * @port: port packet is on
>> + * @length: length of packet
>> + * @broadcast: this packet is a broadcast packet
>> + * @error: this packet has an error
>> + * @sv_frame: indicate if the frame is a SV frame for HSR/PRP
>> + * @lookup_success: src mac found in FDB
>> + * @flood: packet is to be flooded
>> + * @timstamp: Specifies if timestamp is appended to the packet
>
> nit: @timestamp
>

We will address this in the next version.

>> + */
>> +struct prueth_packet_info {
>> + bool start_offset;
>> + bool shadow;
>> + unsigned int port;
>> + unsigned int length;
>> + bool broadcast;
>> + bool error;
>> + bool sv_frame;
>> + bool lookup_success;
>> + bool flood;
>> + bool timestamp;
>> +};
>
> ...

Thanks & Best Regards,
Basharath