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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>-----Original Message-----
>From: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
>Sent: Friday, May 31, 2024 3:52 PM
>To: netdev@xxxxxxxxxxxxxxx
>Cc: nbd@xxxxxxxx; lorenzo.bianconi83@xxxxxxxxx; davem@xxxxxxxxxxxxx;
>edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx;
>conor@xxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx;
>krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx;
>devicetree@xxxxxxxxxxxxxxx; catalin.marinas@xxxxxxx; will@xxxxxxxxxx;
>upstream@xxxxxxxxxx; angelogioacchino.delregno@xxxxxxxxxxxxx;
>benjamin.larsson@xxxxxxxxxx
>Subject: [EXTERNAL] [PATCH net-next 3/3] net: airoha: Introduce ethernet
>support for EN7581 SoC
>
>Prioritize security for external emails: Confirm sender and content safety before
>clicking links or opening attachments
>
>----------------------------------------------------------------------
>Add airoha_eth driver in order to introduce ethernet support for
>Airoha EN7581 SoC available on EN7581 development board (en7581-evb).
>en7581-evb networking architecture is composed by airoha_eth as mac
>controller (cpu port) and a mt7530 dsa based switch.
>EN7581 mac controller is mainly composed by Frame Engine (FE) and
>QoS-DMA (QDMA) modules. FE is used for traffic offloading (just basic
>functionalities are supported now) while QDMA is used for DMA operation
>and QOS functionalities between mac layer and the dsa switch (hw QoS is
>not available yet and it will be added in the future).
>Currently only hw lan features are available, hw wan will be added with
>subsequent patches.
>
>Tested-by: Benjamin Larsson <benjamin.larsson@xxxxxxxxxx>
>Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
>---
......
>+
>+static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
>+{
>+	struct airoha_eth *eth = q->eth;
>+	struct device *dev = eth->net_dev->dev.parent;
>+	int done = 0, qid = q - &eth->q_rx[0];
>+
>+	spin_lock_bh(&q->lock);

There is one napi per queue, why lock ?

...........................
>+
>+	q = &eth->q_tx[qid];
>+	spin_lock_bh(&q->lock);

Same here, is this lock needed ?
If yes, can you please elaborate why.

>+
>+	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 ??

...........
>+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 ?


Overall this is a big patch deserving to be split, probably separate patches for init and datapath logic.
Also I do not see basic functionality like BQL not being supported, is that intentional ?

Thanks,
Sunil.






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux