[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<BY3PR18MB4737F74D6674C04CAFDCD9C9C6FF2@BY3PR18MB4737.namprd18.prod.outlook.com>
Date: Mon, 3 Jun 2024 05:19:29 +0000
From: Sunil Kovvuri Goutham <sgoutham@...vell.com>
To: Lorenzo Bianconi <lorenzo@...nel.org>,
"netdev@...r.kernel.org"
<netdev@...r.kernel.org>
CC: "nbd@....name" <nbd@....name>,
"lorenzo.bianconi83@...il.com"
<lorenzo.bianconi83@...il.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org"
<kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"conor@...nel.org" <conor@...nel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"robh+dt@...nel.org"
<robh+dt@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org"
<krzysztof.kozlowski+dt@...aro.org>,
"conor+dt@...nel.org"
<conor+dt@...nel.org>,
"devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>,
"catalin.marinas@....com"
<catalin.marinas@....com>,
"will@...nel.org" <will@...nel.org>,
"upstream@...oha.com" <upstream@...oha.com>,
"angelogioacchino.delregno@...labora.com"
<angelogioacchino.delregno@...labora.com>,
"benjamin.larsson@...exis.eu"
<benjamin.larsson@...exis.eu>
Subject: RE: [EXTERNAL] [PATCH net-next 3/3] net: airoha: Introduce ethernet
support for EN7581 SoC
>-----Original Message-----
>From: Lorenzo Bianconi <lorenzo@...nel.org>
>Sent: Friday, May 31, 2024 3:52 PM
>To: netdev@...r.kernel.org
>Cc: nbd@....name; lorenzo.bianconi83@...il.com; davem@...emloft.net;
>edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com;
>conor@...nel.org; linux-arm-kernel@...ts.infradead.org; robh+dt@...nel.org;
>krzysztof.kozlowski+dt@...aro.org; conor+dt@...nel.org;
>devicetree@...r.kernel.org; catalin.marinas@....com; will@...nel.org;
>upstream@...oha.com; angelogioacchino.delregno@...labora.com;
>benjamin.larsson@...exis.eu
>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@...exis.eu>
>Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
>---
......
>+
>+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.
Powered by blists - more mailing lists