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

[Yibo Dong]

On Mon, Oct 05, 2026 at 07:36:05AM +0000, netdev-bot+sashiko@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] rnpgbe_probe() now calls dma_set_mask_and_coherent(&pdev->dev,
>   DMA_BIT_MASK(56)) and ignores the return value.
> - [Low] rnpgbe_add_adapter() calls netif_carrier_off(netdev) just before
>   register_netdev().
> 
> --
> 
> net: rnpgbe: Add basic TX packet transmission support
> 
> This commit adds the TX path for the rnpgbe driver: TX descriptor and
> buffer management, rnpgbe_xmit_frame_ring(), ring setup and teardown, and
> TX completion handling in rnpgbe_clean_tx_irq(). It also keeps the
> carrier off at registration, sets 56-bit streaming and coherent DMA masks,
> and enables NETIF_F_HIGHDMA when the mask allows it.
> 
> > 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
> 
> [ ... ]
> 
> > @@ -248,12 +304,7 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> >  	if (err)
> >  		return err;
> >  
> > -	err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(56));
> > -	if (err) {
> > -		dev_err(&pdev->dev,
> > -			"No usable DMA configuration, aborting %d\n", err);
> > -		goto err_disable_dev;
> > -	}
> > +	dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(56));
> 
> [Severity: Medium]
> Should the return value of dma_set_mask_and_coherent() still be checked
> here?
> 
> Before this patch, rnpgbe_probe() logged "No usable DMA configuration,
> aborting" and jumped to err_disable_dev when setting the mask failed. With
> this change, a failure is silently ignored:
> 
> include/linux/dma-mapping.h:dma_set_mask_and_coherent() {
> 	int rc = dma_set_mask(dev, mask);
> 	if (rc == 0)
> 		dma_set_coherent_mask(dev, mask);
> 	return rc;
> }
> 
> If dma_set_mask() fails, neither mask is set. Both stay at the PCI default
> and probe goes on to register the netdev. The only visible effect is that
> the dma_get_mask() check in rnpgbe_add_adapter() quietly leaves out
> NETIF_F_HIGHDMA, with no diagnostic for the user.
> 
> The commit message describes this hunk as "Configure streaming and
> coherent DMA masks" and does not mention that the error check was
> removed. The call is still unchecked at the end of the series.
> 
I left the return value unchecked because dma_set_mask_and_coherent()
does not fail for masks wider than 32 bits.
Similar feedback for other patches point this out:
https://lore.kernel.org/netdev/20260814123330.4b8056f5@xxxxxxxxxx/
> >  
> >  	err = pci_request_mem_regions(pdev, rnpgbe_driver_name);
> >  	if (err) {
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/B438B0A83ACCA2DE%2B20261004065604.224873-1-dong100%40mucse.com
>