>-----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 - ð->q_rx[0]; >+ >+ spin_lock_bh(&q->lock); There is one napi per queue, why lock ? ........................... >+ >+ q = ð->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.