> -----Original Message----- > From: Subbaraya Sundeep Bhatta <sbhatta@xxxxxxxxxxx> > Sent: Wednesday, May 10, 2023 7:14 PM > To: Gaddam, Sarath Babu Naidu > <sarath.babu.naidu.gaddam@xxxxxxx>; davem@xxxxxxxxxxxxx; > edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; > robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx > Cc: linux@xxxxxxxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>; > Pandey, Radhey Shyam <radhey.shyam.pandey@xxxxxxx>; > netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Sarangi, > Anirudha <anirudha.sarangi@xxxxxxx>; Katakam, Harini > <harini.katakam@xxxxxxx>; git (AMD-Xilinx) <git@xxxxxxx> > Subject: RE: [EXT] [PATCH net-next V3 3/3] net: axienet: Introduce > dmaengine support > > Hi, > > >-----Original Message----- > >From: Sarath Babu Naidu Gaddam > <sarath.babu.naidu.gaddam@xxxxxxx> > >Sent: Wednesday, May 10, 2023 2:21 PM > >To: davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; > >pabeni@xxxxxxxxxx; robh+dt@xxxxxxxxxx; > >krzysztof.kozlowski+dt@xxxxxxxxxx > >Cc: linux@xxxxxxxxxxxxxxx; michal.simek@xxxxxxx; > >radhey.shyam.pandey@xxxxxxx; netdev@xxxxxxxxxxxxxxx; > >devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > >linux- kernel@xxxxxxxxxxxxxxx; anirudha.sarangi@xxxxxxx; > >harini.katakam@xxxxxxx; sarath.babu.naidu.gaddam@xxxxxxx; > git@xxxxxxx > >Subject: [PATCH net-next V3 3/3] net: axienet: Introduce dmaengine > >support > > > >Add dmaengine framework to communicate with the xilinx DMAengine > >driver(AXIDMA). > > > >Axi ethernet driver uses separate channels for transmit and receive. > >Add support for these channels to handle TX and RX with skb and > >appropriate callbacks. Also add axi ethernet core interrupt for > >dmaengine framework support. > > > >The dmaengine framework was extended for metadata API support > during > >the axidma RFC[1] discussion. However it still needs further > >enhancements to make it well suited for ethernet usecases. The ethernet > >features i.e ethtool set/get of DMA IP properties, ndo_poll_controller, > >trigger reset of DMA IP from ethernet are not supported (mentioned in > >TODO) and it requires follow-up discussion and dma framework > enhancement. > > > >[1]: https://urldefense.proofpoint.com/v2/url?u=https- > >3A__lore.kernel.org_lkml_1522665546-2D10035-2D1-2Dgit-2Dsend- > 2Demail- > >2D&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=wYboOaw70DU5hR > M5HDwOR > >Jx_MfD- > hXXKii2eobNikgU&m=qvFFGXjULUP3IPFOil5aKySdkumrp8V0TYK4kQ- > >yzsWPmRolFaQvfKMhaR11_dZv&s=brEWb1Til18VCodb- > H4tST0HOBXKIJtL- > >2ztGmMZz_8&e= > >radheys@xxxxxxxxxx > > > >Signed-off-by: Radhey Shyam Pandey > <radhey.shyam.pandey@xxxxxxxxxx> > >Signed-off-by: Sarath Babu Naidu Gaddam > ><sarath.babu.naidu.gaddam@xxxxxxx> > >--- > >Performance numbers(Mbps): > > > > | TCP | UDP | > > ----------------- > > | Tx | 920 | 800 | > > ----------------- > > | Rx | 620 | 910 | > > > >Changes in V3: > >1) New patch for dmaengine framework support. > >--- > > drivers/net/ethernet/xilinx/xilinx_axienet.h | 6 + > > .../net/ethernet/xilinx/xilinx_axienet_main.c | 331 > +++++++++++++++++- > > 2 files changed, 335 insertions(+), 2 deletions(-) > > > >diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h > >b/drivers/net/ethernet/xilinx/xilinx_axienet.h > >index 10917d997d27..fbe00c5390d5 100644 > >--- a/drivers/net/ethernet/xilinx/xilinx_axienet.h > >+++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h > >@@ -436,6 +436,9 @@ struct axidma_bd { > > * @coalesce_count_tx: Store the irq coalesce on TX side. > > * @coalesce_usec_tx: IRQ coalesce delay for TX > > * @has_dmas: flag to check dmaengine framework usage. > >+ * @tx_chan: TX DMA channel. > >+ * @rx_chan: RX DMA channel. > >+ * @skb_cache: Custom skb slab allocator > > */ > > struct axienet_local { > > struct net_device *ndev; > >@@ -501,6 +504,9 @@ struct axienet_local { > > u32 coalesce_count_tx; > > u32 coalesce_usec_tx; > > u8 has_dmas; > >+ struct dma_chan *tx_chan; > >+ struct dma_chan *rx_chan; > >+ struct kmem_cache *skb_cache; > > }; > > > > /** > >diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >index 8678fc09245a..662c77ff0e99 100644 > >--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >@@ -37,6 +37,9 @@ > > #include <linux/phy.h> > > #include <linux/mii.h> > > #include <linux/ethtool.h> > >+#include <linux/dmaengine.h> > >+#include <linux/dma-mapping.h> > >+#include <linux/dma/xilinx_dma.h> > > > > #include "xilinx_axienet.h" > > > >@@ -46,6 +49,9 @@ > > #define TX_BD_NUM_MIN (MAX_SKB_FRAGS + 1) > > #define TX_BD_NUM_MAX 4096 > > #define RX_BD_NUM_MAX 4096 > >+#define DMA_NUM_APP_WORDS 5 > >+#define LEN_APP 4 > >+#define RX_BUF_NUM_DEFAULT 128 > > > > /* Must be shorter than length of ethtool_drvinfo.driver field to fit */ > > #define DRIVER_NAME "xaxienet" > >@@ -56,6 +62,16 @@ > > > > #define AXIENET_USE_DMA(lp) ((lp)->has_dmas) > > > >+struct axi_skbuff { > >+ struct scatterlist sgl[MAX_SKB_FRAGS + 1]; > >+ struct dma_async_tx_descriptor *desc; > >+ dma_addr_t dma_address; > >+ struct sk_buff *skb; > >+ int sg_len; > >+} __packed; > >+ > >+static int axienet_rx_submit_desc(struct net_device *ndev); > >+ > > /* Match table for of_platform binding */ static const struct > >of_device_id axienet_of_match[] = { > > { .compatible = "xlnx,axi-ethernet-1.00.a", }, @@ -728,6 > +744,108 @@ > >static inline int axienet_check_tx_bd_space(struct axienet_local *lp, > > return 0; > > } > > > >+/** > >+ * axienet_dma_tx_cb - DMA engine callback for TX channel. > >+ * @data: Pointer to the axi_skbuff structure > >+ * @result: error reporting through dmaengine_result. > >+ * This function is called by dmaengine driver for TX channel to > >+notify > >+ * that the transmit is done. > >+ */ > >+static void axienet_dma_tx_cb(void *data, const struct > >+dmaengine_result > >*result) > >+{ > >+ struct axi_skbuff *axi_skb = data; > >+ > >+ struct net_device *netdev = axi_skb->skb->dev; > >+ struct axienet_local *lp = netdev_priv(netdev); > >+ > >+ u64_stats_update_begin(&lp->tx_stat_sync); > >+ u64_stats_add(&lp->tx_bytes, axi_skb->skb->len); > >+ u64_stats_add(&lp->tx_packets, 1); > >+ u64_stats_update_end(&lp->tx_stat_sync); > >+ > >+ dma_unmap_sg(lp->dev, axi_skb->sgl, axi_skb->sg_len, > >DMA_MEM_TO_DEV); > >+ dev_kfree_skb_any(axi_skb->skb); > >+ kmem_cache_free(lp->skb_cache, axi_skb); } > >+ > >+/** > >+ * axienet_start_xmit_dmaengine - Starts the transmission. > >+ * @skb: sk_buff pointer that contains data to be Txed. > >+ * @ndev: Pointer to net_device structure. > >+ * > >+ * Return: NETDEV_TX_OK, on success > >+ * NETDEV_TX_BUSY, if any memory failure or SG error. > >+ * > >+ * This function is invoked from xmit to initiate transmission. The > >+ * function sets the skbs , call back API, SG etc. > >+ * Additionally if checksum offloading is supported, > >+ * it populates AXI Stream Control fields with appropriate values. > >+ */ > >+static netdev_tx_t > >+axienet_start_xmit_dmaengine(struct sk_buff *skb, struct net_device > >+*ndev) { > >+ struct dma_async_tx_descriptor *dma_tx_desc = NULL; > >+ struct axienet_local *lp = netdev_priv(ndev); > >+ u32 app[DMA_NUM_APP_WORDS] = {0}; > >+ struct axi_skbuff *axi_skb; > >+ u32 csum_start_off; > >+ u32 csum_index_off; > >+ int sg_len; > >+ int ret; > >+ > >+ sg_len = skb_shinfo(skb)->nr_frags + 1; > >+ axi_skb = kmem_cache_zalloc(lp->skb_cache, GFP_KERNEL); > >+ if (!axi_skb) > >+ return NETDEV_TX_BUSY; > >+ > >+ sg_init_table(axi_skb->sgl, sg_len); > >+ ret = skb_to_sgvec(skb, axi_skb->sgl, 0, skb->len); > >+ if (unlikely(ret < 0)) > >+ goto xmit_error_skb_sgvec; > >+ > >+ ret = dma_map_sg(lp->dev, axi_skb->sgl, sg_len, > DMA_TO_DEVICE); > >+ if (ret == 0) > >+ goto xmit_error_skb_sgvec; > >+ > >+ /*Fill up app fields for checksum */ > >+ if (skb->ip_summed == CHECKSUM_PARTIAL) { > >+ if (lp->features & XAE_FEATURE_FULL_TX_CSUM) { > >+ /* Tx Full Checksum Offload Enabled */ > >+ app[0] |= 2; > >+ } else if (lp->features & > XAE_FEATURE_PARTIAL_RX_CSUM) { > >+ csum_start_off = skb_transport_offset(skb); > >+ csum_index_off = csum_start_off + skb- > >csum_offset; > >+ /* Tx Partial Checksum Offload Enabled */ > >+ app[0] |= 1; > >+ app[1] = (csum_start_off << 16) | > csum_index_off; > >+ } > >+ } else if (skb->ip_summed == CHECKSUM_UNNECESSARY) { > >+ app[0] |= 2; /* Tx Full Checksum Offload Enabled */ > >+ } > >+ > >+ dma_tx_desc = lp->tx_chan->device->device_prep_slave_sg(lp- > >tx_chan, > >axi_skb->sgl, > >+ sg_len, DMA_MEM_TO_DEV, > >+ DMA_PREP_INTERRUPT, (void *)app); > >+ > >+ if (!dma_tx_desc) > >+ goto xmit_error_prep; > >+ > >+ axi_skb->skb = skb; > >+ axi_skb->sg_len = sg_len; > >+ dma_tx_desc->callback_param = axi_skb; > >+ dma_tx_desc->callback_result = axienet_dma_tx_cb; > >+ dmaengine_submit(dma_tx_desc); > >+ dma_async_issue_pending(lp->tx_chan); > >+ > >+ return NETDEV_TX_OK; > >+ > >+xmit_error_prep: > >+ dma_unmap_sg(lp->dev, axi_skb->sgl, sg_len, DMA_TO_DEVICE); > >+xmit_error_skb_sgvec: > >+ kmem_cache_free(lp->skb_cache, axi_skb); > >+ return NETDEV_TX_BUSY; > >+} > >+ > > /** > > * axienet_tx_poll - Invoked once a transmit is completed by the > > * Axi DMA Tx channel. > >@@ -912,7 +1030,42 @@ axienet_start_xmit(struct sk_buff *skb, struct > >net_device *ndev) > > if (!AXIENET_USE_DMA(lp)) > > return axienet_start_xmit_legacy(skb, ndev); > > else > >- return NETDEV_TX_BUSY; > >+ return axienet_start_xmit_dmaengine(skb, ndev); > > You can avoid this if else by > if (AXIENET_USE_DMA(lp)) > return axienet_start_xmit_dmaengine(skb, ndev); > > and no need of defining axienet_start_xmit_legacy function in patch 2/3. > _legacy may not be correct since you support both in-built dma or with > dma engine just by turning on/off dt properties. Also does this driver roll > back to using in-built dma if DMA_ENGINE is not compiled in? We wanted to separate it to support future maintenance or deprecation with a separate wrapper. It is only a preference and either approach is ok. There is no fallback to built in DMA. The intention is to use dmaengine as the preferred approach and eventually deprecate built-in dma. If DMA_ENGINE is not present, the dma channel request fails through the usual exit path. Will address remaining review comments of 2/3 and 3/3 in the next Version. Thanks, Sarath
