[PATCH net-next 3/3] net: airoha: Introduce ethernet support for EN7581 SoC

[Sunil Kovvuri Goutham]

>-----Original Message-----
>From: Lorenzo Bianconi <lorenzo at kernel.org>
>Sent: Monday, June 3, 2024 1:24 PM
>To: Sunil Kovvuri Goutham <sgoutham at marvell.com>
>Cc: netdev at vger.kernel.org; nbd at nbd.name; lorenzo.bianconi83 at gmail.com;
>davem at davemloft.net; edumazet at google.com; kuba at kernel.org;
>pabeni at redhat.com; conor at kernel.org; linux-arm-kernel at lists.infradead.org;
>robh+dt at kernel.org; krzysztof.kozlowski+dt at linaro.org; conor+dt at kernel.org;
>devicetree at vger.kernel.org; catalin.marinas at arm.com; will at kernel.org;
>upstream at airoha.com; angelogioacchino.delregno at collabora.com;
>benjamin.larsson at genexis.eu
>Subject: Re: [EXTERNAL] [PATCH net-next 3/3] net: airoha: Introduce ethernet
>support for EN7581 SoC
>
>> >+
>> >+	q = &eth->q_tx[qid];
>> >+	spin_lock_bh(&q->lock);
>>
>> Same here, is this lock needed ?
>> If yes, can you please elaborate why.
>
>ndo_start_xmit callback can run in parallel with airoha_qdma_tx_napi_poll()
>

Okay.

>>
>> >+
>> >+	if (q->queued + nr_frags > q->ndesc) {
>> >+		/* not enough space in the queue */
>> >+		spin_unlock_bh(&q->lock);
>> >+		return NETDEV_TX_BUSY;
>> >+	}
>> >+
>>
>> I do not see netif_set_tso_max_segs() being set, so HW doesn't have
>> any limit wrt number of TSO segs and number of fragments in skb, is it ??
>
>I do not think there is any specific limitation for it
>

Okay.

>>
>> ...........
>> >+static int airoha_probe(struct platform_device *pdev) {
>> >+	struct device_node *np = pdev->dev.of_node;
>> >+	struct net_device *dev;
>> >+	struct airoha_eth *eth;
>> >+	int err;
>> >+
>> >+	dev = devm_alloc_etherdev_mqs(&pdev->dev, sizeof(*eth),
>> >+				      AIROHA_NUM_TX_RING,
>> >AIROHA_NUM_RX_RING);
>>
>> Always 32 queues, even if kernel is booted with less number cores ?
>
>ethtool is not supported yet, I will add it with followup patches
>

I meant by default.
Wouldn't it better to check online cpu count ?
If system is booted with just 10 cores, then why initialize 32 cores is my point.

Thanks,
Sunil.