Re: [PATCH net-next v14 2/5] net: rnpgbe: Add basic TX packet transmission support

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] rnpgbe_add_adapter() now calls netif_carrier_off() before
  register_netdev() (rnpgbe_main.c:264), and nothing at this commit…

--

net: rnpgbe: Add basic TX packet transmission support

This commit adds the basic TX path for the rnpgbe driver: TX descriptors,
buffer management, ring setup and teardown, and TX completion handling
from NAPI. It also keeps per-queue TX statistics in adapter-owned storage
and sets 56-bit streaming and coherent DMA masks.

> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> index 6022a1a2ddf1e..265a3fcb07beb 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c

[ ... ]

> @@ -206,6 +254,14 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
>  		goto err_clear_interrupt;
>  	}
>  
> +	netdev->features |= NETIF_F_SG;
> +	netdev->hw_features |= NETIF_F_SG;
> +	if (dma_get_mask(&pdev->dev) > DMA_BIT_MASK(32)) {
> +		netdev->features |= NETIF_F_HIGHDMA;
> +		netdev->hw_features |= NETIF_F_HIGHDMA;
> +	}
> +
> +	netif_carrier_off(netdev);
>  	err = register_netdev(netdev);
>  	if (err)
>  		goto err_remove_mbx;

[Severity: Low]
rnpgbe_add_adapter() now calls netif_carrier_off(). At this commit, does
anything turn the carrier back on? I couldn't find a netif_carrier_on()
call anywhere in the driver. On a successful open, rnpgbe_up_complete()
only starts the TX queues:

rnpgbe_up_complete()
    rnpgbe_configure_msix(mucse);
    rnpgbe_napi_enable_all(mucse);
    rnpgbe_irq_enable(mucse);
    netif_tx_start_all_queues(netdev);

While the carrier is off, dev_activate() defers qdisc activation. So the
interface shows NO-CARRIER after open, and the new TX path looks
unreachable at this commit:

rnpgbe_xmit_frame()->rnpgbe_xmit_frame_ring()->rnpgbe_tx_map()

The commit message also doesn't mention the carrier change.

The later patch "net: rnpgbe: Add link status handling support" appears
to add netif_carrier_on() through rnpgbe_link_is_up(), so this is fixed
later in the series. Before this patch, rnpgbe_xmit_frame() dropped every
skb anyway, so TX doesn't get any worse.

Could netif_carrier_off() move into the link handling patch? Or could this
commit message mention it, so the intermediate state is clear when
bisecting?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/0A4D45AD9F6A0F14%2B20260928033701.1033196-1-dong100%40mucse.com