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

Basharath Hussain Khaja
2025-01-23T12:30:29Z

> Le 09/01/2025 à 11:55, Basharath Hussain Khaja a écrit :
>> 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.
>
> Hi,
> a few nitpicks.
>
> ...
>
>> +static int icssm_prueth_tx_enqueue(struct prueth_emac *emac,
>> + struct sk_buff *skb,
>> + enum prueth_queue_id queue_id)
>> +{
>> + struct prueth_queue_desc __iomem *queue_desc;
>> + const struct prueth_queue_info *txqueue;
>> + u16 bd_rd_ptr, bd_wr_ptr, update_wr_ptr;
>> + struct net_device *ndev = emac->ndev;
>> + unsigned int buffer_desc_count;
>> + int free_blocks, update_block;
>> + bool buffer_wrapped = false;
>> + int write_block, read_block;
>> + void *src_addr, *dst_addr;
>> + int pkt_block_size;
>> + void __iomem *dram;
>> + int txport, pktlen;
>> + u32 wr_buf_desc;
>> + void *ocmc_ram;
>> +
>> + dram = emac->prueth->mem[emac->dram].va;
>> + if (eth_skb_pad(skb)) {
>> + if (netif_msg_tx_err(emac) && net_ratelimit())
>> + netdev_err(ndev, "packet pad failed");
>
> Missing trailing \n.
>
>> + return -ENOMEM;
>> + }
>> +
>> + /* which port to tx: MII0 or MII1 */
>> + txport = emac->tx_port_queue;
>
> ...
>
>> +static int icssm_emac_request_irqs(struct prueth_emac *emac)
>> +{
>> + struct net_device *ndev = emac->ndev;
>> + int ret = 0;
>
> No need to init.
>
>> +
>> + ret = request_threaded_irq(emac->rx_irq, NULL, icssm_emac_rx_thread,
>> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
>> + ndev->name, ndev);
>> + if (ret) {
>> + netdev_err(ndev, "unable to request RX IRQ\n");
>> + return ret;
>> + }
>
> ...
>
>> +static int icssm_emac_ndo_start_xmit(struct sk_buff *skb,
>> + struct net_device *ndev)
>> +{
>> + struct prueth_emac *emac = netdev_priv(ndev);
>> + int ret = 0;
>> + u16 qid;
>> +
>> + if (unlikely(!emac->link)) {
>> + if (netif_msg_tx_err(emac) && net_ratelimit())
>> + netdev_err(ndev, "No link to transmit");
>
> \n
>
>> + goto fail_tx;
>> + }
>> +
>> + qid = icssm_prueth_get_tx_queue_id(emac->prueth, skb);...

We will handle all comments in the next version.

Thanks & Best Regards,
Basharath