Re: [PATCH V10 net-next 06/10] net: hibmcge: Implement .ndo_start_xmit function

[Christophe JAILLET]

Le 12/09/2024 à 04:51, Jijie Shao a écrit :
> Implement .ndo_start_xmit function to fill the information of the packet
> to be transmitted into the tx descriptor, and then the hardware will
> transmit the packet using the information in the tx descriptor.
> In addition, we also implemented the tx_handler function to enable the
> tx descriptor to be reused, and .ndo_tx_timeout function to print some
> information when the hardware is busy.
>
> Signed-off-by: Jijie Shao <shaojijie@xxxxxxxxxx>

...

> +static int hbg_dma_map(struct hbg_buffer *buffer)
> +{
> +	struct hbg_priv *priv = buffer->priv;
> +
> +	buffer->skb_dma = dma_map_single(&priv->pdev->dev,
> +					 buffer->skb->data, buffer->skb_len,
> +					 buffer_to_dma_dir(buffer));
> +	if (unlikely(dma_mapping_error(&priv->pdev->dev, buffer->skb_dma)))

I don't think that the unlikely() is needed.

Maybe some other ones are also not needed when in slow path.

> +		return -ENOMEM;
> +
> +	return 0;
> +}

...

> +static int hbg_tx_ring_init(struct hbg_priv *priv)
> +{
> +	struct hbg_ring *tx_ring = &priv->tx_ring;
> +
> +	if (!tx_ring->tout_log_buf)
> +		tx_ring->tout_log_buf = devm_kzalloc(&priv->pdev->dev,
> +						     HBG_TX_TIMEOUT_BUF_LEN,
> +						     GFP_KERNEL);

Nitpick: devm_kmalloc() looks enough.
It is zeroed only the first time we get there anyway.

> +
> +	if (!tx_ring->tout_log_buf)
> +		return -ENOMEM;
> +
> +	return hbg_ring_init(priv, tx_ring, hbg_napi_tx_recycle, HBG_DIR_TX);
> +}