The axiethernet driver can use the dmaengine framework to communicate
with the xilinx DMAengine driver(AXIDMA, MCDMA). The inspiration behind
this dmaengine adoption is to reuse the in-kernel xilinx dma engine
driver[1] and remove redundant dma programming sequence[2] from the
ethernet driver. This simplifies the ethernet driver and also makes
it generic to be hooked to any complaint dma IP i.e AXIDMA, MCDMA
without any modification.
The dmaengine framework was extended for metadata API support during
the axidma RFC[3] discussion. However, it still needs further
enhancements to make it well suited for ethernet usecases.
Comments, suggestions, thoughts to implement remaining functional
features are very welcome!
[1]: https://github.com/torvalds/linux/blob/master/drivers/dma/xilinx/xilinx_dma.c
[2]: https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/xilinx/xilinx_axienet_main.c#L238
[3]: http://lkml.iu.edu/hypermail/linux/kernel/1804.0/00367.html
[4]: https://lore.kernel.org/all/[email protected]
Changes in v6:
- Remove patchset 1-7 as it was applied to dmaengine tree in v5 version.
- Added Krzysztof reviewed-by tag for dmaengine binding patch.
- Rename struct axi_skbuff to skbuf_dma_descriptor and removed
__packed attribute.
- Drop kmem_cache implementation and switch to using ring buffers.
- Remove __inline from axienet_init_dmaengine().
- Name labels after the target.
- Add error check for platform_get_irq_optional().
- Fix double space and no empty lines between call and its error check.
Changes in v5:
- Fix git am failure on net-next
- Addressed DT binding review comments i.e Modified commit description to
remove dmaengine framework references and instead describe how
axiethernet IP uses DMA channels.
- Fix "^[tr]x_chan[0-9]|1[0-5]$" -> "^[tr]x_chan([0-9]|1[0-5])$"
- Drop generic dmas description.
- Fix kmem_cache resource leak.
- Merge Xilinx DMA enhancements and optimization[4] into this series.
Changes in V4:
- Updated commit description about tx/rx channels name(1/3).
- Removed "dt-bindings" and "dmaengine" strings in subject(1/3).
- Extended dmas and dma-names to support MCDMA channel names(1/3).
- Rename has_dmas to use_dmaegine(2/3).
- Remove the AXIENET_USE_DMA(2/3).
- Remove the AXIENET_USE_DMA(3/3).
- Add dev_err_probe for dma_request_chan error handling(3/3).
- Add kmem_cache_destroy for create in axienet_setup_dma_chan(3/3).
Changes in V3:
- Moved RFC to PATCH.
- Removed ethtool get/set coalesce, will be added later.
- Added backward comapatible support.
- Split the dmaengine support patch of V2 into two patches(2/3 and 3/3).
https://lore.kernel.org/all/[email protected]/
Changes in V2:
- Add ethtool get/set coalesce and DMA reset using DMAengine framework.
- Add performance numbers.
- Remove .txt and change the name of file to xlnx,axiethernet.yaml.
- Fix DT check warning(Fix DT check warning('device_type' does not match
any of the regexes:'pinctrl-[0-9]+' From schema: Documentation/
devicetree/bindings/net/xilinx_axienet.yaml).
Radhey Shyam Pandey (2):
dt-bindings: net: xlnx,axi-ethernet: Introduce DMA support
net: axienet: Introduce dmaengine support
Sarath Babu Naidu Gaddam (1):
net: axienet: Preparatory changes for dmaengine support
.../bindings/net/xlnx,axi-ethernet.yaml | 16 +
drivers/net/ethernet/xilinx/Kconfig | 1 +
drivers/net/ethernet/xilinx/xilinx_axienet.h | 35 +
.../net/ethernet/xilinx/xilinx_axienet_main.c | 661 ++++++++++++++----
4 files changed, 590 insertions(+), 123 deletions(-)
--
2.34.1
From: Sarath Babu Naidu Gaddam <[email protected]>
The axiethernet driver has inbuilt dma programming. In order to add
dmaengine support and make it's integration seamless the current axidma
inbuilt programming code is put under use_dmaengine check.
It also performs minor code reordering to minimize conditional
use_dmaengine checks and there is no functional change. It uses
"dmas" property to identify whether it should use a dmaengine
framework or inbuilt axidma programming.
Signed-off-by: Sarath Babu Naidu Gaddam <[email protected]>
Signed-off-by: Radhey Shyam Pandey <[email protected]>
---
Changes for v6:
- None
Changes for v5:
- Fix git apply failure due to commit
f1bc9fc4a06de0108e0dca2a9a7e99ba1fc632f9
Changes for v4:
- Renamed has_dmas to use_dmaegine.
- Removed the AXIENET_USE_DMA.
- Changed the start_xmit_** functions description.
Changes for v3:
- New patch
---
drivers/net/ethernet/xilinx/xilinx_axienet.h | 2 +
.../net/ethernet/xilinx/xilinx_axienet_main.c | 313 +++++++++++-------
2 files changed, 188 insertions(+), 127 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h
index 575ff9de8985..3ead0bac597b 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet.h
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h
@@ -435,6 +435,7 @@ struct axidma_bd {
* @coalesce_usec_rx: IRQ coalesce delay for RX
* @coalesce_count_tx: Store the irq coalesce on TX side.
* @coalesce_usec_tx: IRQ coalesce delay for TX
+ * @use_dmaengine: flag to check dmaengine framework usage.
*/
struct axienet_local {
struct net_device *ndev;
@@ -499,6 +500,7 @@ struct axienet_local {
u32 coalesce_usec_rx;
u32 coalesce_count_tx;
u32 coalesce_usec_tx;
+ u8 use_dmaengine;
};
/**
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index b7ec4dafae90..67901700e296 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -589,10 +589,6 @@ static int axienet_device_reset(struct net_device *ndev)
struct axienet_local *lp = netdev_priv(ndev);
int ret;
- ret = __axienet_device_reset(lp);
- if (ret)
- return ret;
-
lp->max_frm_size = XAE_MAX_VLAN_FRAME_SIZE;
lp->options |= XAE_OPTION_VLAN;
lp->options &= (~XAE_OPTION_JUMBO);
@@ -606,11 +602,17 @@ static int axienet_device_reset(struct net_device *ndev)
lp->options |= XAE_OPTION_JUMBO;
}
- ret = axienet_dma_bd_init(ndev);
- if (ret) {
- netdev_err(ndev, "%s: descriptor allocation failed\n",
- __func__);
- return ret;
+ if (!lp->use_dmaengine) {
+ ret = __axienet_device_reset(lp);
+ if (ret)
+ return ret;
+
+ ret = axienet_dma_bd_init(ndev);
+ if (ret) {
+ netdev_err(ndev, "%s: descriptor allocation failed\n",
+ __func__);
+ return ret;
+ }
}
axienet_status = axienet_ior(lp, XAE_RCW1_OFFSET);
@@ -776,20 +778,20 @@ static int axienet_tx_poll(struct napi_struct *napi, int budget)
}
/**
- * axienet_start_xmit - Starts the transmission.
+ * axienet_start_xmit_legacy - 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 of the descriptors are not free
*
- * This function is invoked from upper layers to initiate transmission. The
+ * This function is invoked from axienet_start_xmit to initiate transmission. The
* function uses the next available free BDs and populates their fields to
* start the transmission. Additionally if checksum offloading is supported,
* it populates AXI Stream Control fields with appropriate values.
*/
static netdev_tx_t
-axienet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+axienet_start_xmit_legacy(struct sk_buff *skb, struct net_device *ndev)
{
u32 ii;
u32 num_frag;
@@ -891,6 +893,27 @@ axienet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
return NETDEV_TX_OK;
}
+/**
+ * axienet_start_xmit - Invoke the transmission function
+ * @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 of the descriptors are not free
+ *
+ * This function is invoked from upper layers to initiate transmission
+ */
+static netdev_tx_t
+axienet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+ struct axienet_local *lp = netdev_priv(ndev);
+
+ if (!lp->use_dmaengine)
+ return axienet_start_xmit_legacy(skb, ndev);
+ else
+ return NETDEV_TX_BUSY;
+}
+
/**
* axienet_rx_poll - Triggered by RX ISR to complete the BD processing.
* @napi: Pointer to NAPI structure.
@@ -1125,41 +1148,22 @@ static irqreturn_t axienet_eth_irq(int irq, void *_ndev)
static void axienet_dma_err_handler(struct work_struct *work);
/**
- * axienet_open - Driver open routine.
- * @ndev: Pointer to net_device structure
+ * axienet_init_legacy_dma - init the dma legacy code.
+ * @ndev: Pointer to net_device structure
*
* Return: 0, on success.
- * non-zero error value on failure
+ * non-zero error value on failure
+ *
+ * This is the dma initialization code. It also allocates interrupt
+ * service routines, enables the interrupt lines and ISR handling.
*
- * This is the driver open routine. It calls phylink_start to start the
- * PHY device.
- * It also allocates interrupt service routines, enables the interrupt lines
- * and ISR handling. Axi Ethernet core is reset through Axi DMA core. Buffer
- * descriptors are initialized.
*/
-static int axienet_open(struct net_device *ndev)
+
+static inline int axienet_init_legacy_dma(struct net_device *ndev)
{
int ret;
struct axienet_local *lp = netdev_priv(ndev);
- dev_dbg(&ndev->dev, "axienet_open()\n");
-
- /* When we do an Axi Ethernet reset, it resets the complete core
- * including the MDIO. MDIO must be disabled before resetting.
- * Hold MDIO bus lock to avoid MDIO accesses during the reset.
- */
- axienet_lock_mii(lp);
- ret = axienet_device_reset(ndev);
- axienet_unlock_mii(lp);
-
- ret = phylink_of_phy_connect(lp->phylink, lp->dev->of_node, 0);
- if (ret) {
- dev_err(lp->dev, "phylink_of_phy_connect() failed: %d\n", ret);
- return ret;
- }
-
- phylink_start(lp->phylink);
-
/* Enable worker thread for Axi DMA error handling */
INIT_WORK(&lp->dma_err_task, axienet_dma_err_handler);
@@ -1193,13 +1197,62 @@ static int axienet_open(struct net_device *ndev)
err_tx_irq:
napi_disable(&lp->napi_tx);
napi_disable(&lp->napi_rx);
- phylink_stop(lp->phylink);
- phylink_disconnect_phy(lp->phylink);
cancel_work_sync(&lp->dma_err_task);
dev_err(lp->dev, "request_irq() failed\n");
return ret;
}
+/**
+ * axienet_open - Driver open routine.
+ * @ndev: Pointer to net_device structure
+ *
+ * Return: 0, on success.
+ * non-zero error value on failure
+ *
+ * This is the driver open routine. It calls phylink_start to start the
+ * PHY device.
+ * It also allocates interrupt service routines, enables the interrupt lines
+ * and ISR handling. Axi Ethernet core is reset through Axi DMA core. Buffer
+ * descriptors are initialized.
+ */
+static int axienet_open(struct net_device *ndev)
+{
+ int ret;
+ struct axienet_local *lp = netdev_priv(ndev);
+
+ dev_dbg(&ndev->dev, "%s\n", __func__);
+
+ /* When we do an Axi Ethernet reset, it resets the complete core
+ * including the MDIO. MDIO must be disabled before resetting.
+ * Hold MDIO bus lock to avoid MDIO accesses during the reset.
+ */
+ axienet_lock_mii(lp);
+ ret = axienet_device_reset(ndev);
+ axienet_unlock_mii(lp);
+
+ ret = phylink_of_phy_connect(lp->phylink, lp->dev->of_node, 0);
+ if (ret) {
+ dev_err(lp->dev, "phylink_of_phy_connect() failed: %d\n", ret);
+ return ret;
+ }
+
+ phylink_start(lp->phylink);
+
+ if (!lp->use_dmaengine) {
+ ret = axienet_init_legacy_dma(ndev);
+ if (ret)
+ goto error_code;
+ }
+
+ return 0;
+
+error_code:
+ phylink_stop(lp->phylink);
+ phylink_disconnect_phy(lp->phylink);
+
+ return ret;
+}
+
/**
* axienet_stop - Driver stop routine.
* @ndev: Pointer to net_device structure
@@ -1216,8 +1269,10 @@ static int axienet_stop(struct net_device *ndev)
dev_dbg(&ndev->dev, "axienet_close()\n");
- napi_disable(&lp->napi_tx);
- napi_disable(&lp->napi_rx);
+ if (!lp->use_dmaengine) {
+ napi_disable(&lp->napi_tx);
+ napi_disable(&lp->napi_rx);
+ }
phylink_stop(lp->phylink);
phylink_disconnect_phy(lp->phylink);
@@ -1225,18 +1280,18 @@ static int axienet_stop(struct net_device *ndev)
axienet_setoptions(ndev, lp->options &
~(XAE_OPTION_TXEN | XAE_OPTION_RXEN));
- axienet_dma_stop(lp);
+ if (!lp->use_dmaengine) {
+ axienet_dma_stop(lp);
+ cancel_work_sync(&lp->dma_err_task);
+ free_irq(lp->tx_irq, ndev);
+ free_irq(lp->rx_irq, ndev);
+ axienet_dma_bd_release(ndev);
+ }
axienet_iow(lp, XAE_IE_OFFSET, 0);
- cancel_work_sync(&lp->dma_err_task);
-
if (lp->eth_irq > 0)
free_irq(lp->eth_irq, ndev);
- free_irq(lp->tx_irq, ndev);
- free_irq(lp->rx_irq, ndev);
-
- axienet_dma_bd_release(ndev);
return 0;
}
@@ -1412,14 +1467,16 @@ static void axienet_ethtools_get_regs(struct net_device *ndev,
data[29] = axienet_ior(lp, XAE_FMI_OFFSET);
data[30] = axienet_ior(lp, XAE_AF0_OFFSET);
data[31] = axienet_ior(lp, XAE_AF1_OFFSET);
- data[32] = axienet_dma_in32(lp, XAXIDMA_TX_CR_OFFSET);
- data[33] = axienet_dma_in32(lp, XAXIDMA_TX_SR_OFFSET);
- data[34] = axienet_dma_in32(lp, XAXIDMA_TX_CDESC_OFFSET);
- data[35] = axienet_dma_in32(lp, XAXIDMA_TX_TDESC_OFFSET);
- data[36] = axienet_dma_in32(lp, XAXIDMA_RX_CR_OFFSET);
- data[37] = axienet_dma_in32(lp, XAXIDMA_RX_SR_OFFSET);
- data[38] = axienet_dma_in32(lp, XAXIDMA_RX_CDESC_OFFSET);
- data[39] = axienet_dma_in32(lp, XAXIDMA_RX_TDESC_OFFSET);
+ if (!lp->use_dmaengine) {
+ data[32] = axienet_dma_in32(lp, XAXIDMA_TX_CR_OFFSET);
+ data[33] = axienet_dma_in32(lp, XAXIDMA_TX_SR_OFFSET);
+ data[34] = axienet_dma_in32(lp, XAXIDMA_TX_CDESC_OFFSET);
+ data[35] = axienet_dma_in32(lp, XAXIDMA_TX_TDESC_OFFSET);
+ data[36] = axienet_dma_in32(lp, XAXIDMA_RX_CR_OFFSET);
+ data[37] = axienet_dma_in32(lp, XAXIDMA_RX_SR_OFFSET);
+ data[38] = axienet_dma_in32(lp, XAXIDMA_RX_CDESC_OFFSET);
+ data[39] = axienet_dma_in32(lp, XAXIDMA_RX_TDESC_OFFSET);
+ }
}
static void
@@ -1880,9 +1937,6 @@ static int axienet_probe(struct platform_device *pdev)
u64_stats_init(&lp->rx_stat_sync);
u64_stats_init(&lp->tx_stat_sync);
- netif_napi_add(ndev, &lp->napi_rx, axienet_rx_poll);
- netif_napi_add(ndev, &lp->napi_tx, axienet_tx_poll);
-
lp->axi_clk = devm_clk_get_optional(&pdev->dev, "s_axi_lite_clk");
if (!lp->axi_clk) {
/* For backward compatibility, if named AXI clock is not present,
@@ -2008,80 +2062,85 @@ static int axienet_probe(struct platform_device *pdev)
goto cleanup_clk;
}
- /* Find the DMA node, map the DMA registers, and decode the DMA IRQs */
- np = of_parse_phandle(pdev->dev.of_node, "axistream-connected", 0);
- if (np) {
- struct resource dmares;
+ if (!of_find_property(pdev->dev.of_node, "dmas", NULL)) {
+ /* Find the DMA node, map the DMA registers, and decode the DMA IRQs */
+ np = of_parse_phandle(pdev->dev.of_node, "axistream-connected", 0);
- ret = of_address_to_resource(np, 0, &dmares);
- if (ret) {
- dev_err(&pdev->dev,
- "unable to get DMA resource\n");
+ if (np) {
+ struct resource dmares;
+
+ ret = of_address_to_resource(np, 0, &dmares);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "unable to get DMA resource\n");
+ of_node_put(np);
+ goto cleanup_clk;
+ }
+ lp->dma_regs = devm_ioremap_resource(&pdev->dev,
+ &dmares);
+ lp->rx_irq = irq_of_parse_and_map(np, 1);
+ lp->tx_irq = irq_of_parse_and_map(np, 0);
of_node_put(np);
+ lp->eth_irq = platform_get_irq_optional(pdev, 0);
+ } else {
+ /* Check for these resources directly on the Ethernet node. */
+ lp->dma_regs = devm_platform_get_and_ioremap_resource(pdev, 1, NULL);
+ lp->rx_irq = platform_get_irq(pdev, 1);
+ lp->tx_irq = platform_get_irq(pdev, 0);
+ lp->eth_irq = platform_get_irq_optional(pdev, 2);
+ }
+ if (IS_ERR(lp->dma_regs)) {
+ dev_err(&pdev->dev, "could not map DMA regs\n");
+ ret = PTR_ERR(lp->dma_regs);
+ goto cleanup_clk;
+ }
+ if (lp->rx_irq <= 0 || lp->tx_irq <= 0) {
+ dev_err(&pdev->dev, "could not determine irqs\n");
+ ret = -ENOMEM;
goto cleanup_clk;
}
- lp->dma_regs = devm_ioremap_resource(&pdev->dev,
- &dmares);
- lp->rx_irq = irq_of_parse_and_map(np, 1);
- lp->tx_irq = irq_of_parse_and_map(np, 0);
- of_node_put(np);
- lp->eth_irq = platform_get_irq_optional(pdev, 0);
- } else {
- /* Check for these resources directly on the Ethernet node. */
- lp->dma_regs = devm_platform_get_and_ioremap_resource(pdev, 1, NULL);
- lp->rx_irq = platform_get_irq(pdev, 1);
- lp->tx_irq = platform_get_irq(pdev, 0);
- lp->eth_irq = platform_get_irq_optional(pdev, 2);
- }
- if (IS_ERR(lp->dma_regs)) {
- dev_err(&pdev->dev, "could not map DMA regs\n");
- ret = PTR_ERR(lp->dma_regs);
- goto cleanup_clk;
- }
- if ((lp->rx_irq <= 0) || (lp->tx_irq <= 0)) {
- dev_err(&pdev->dev, "could not determine irqs\n");
- ret = -ENOMEM;
- goto cleanup_clk;
- }
- /* Reset core now that clocks are enabled, prior to accessing MDIO */
- ret = __axienet_device_reset(lp);
- if (ret)
- goto cleanup_clk;
+ /* Reset core now that clocks are enabled, prior to accessing MDIO */
+ ret = __axienet_device_reset(lp);
+ if (ret)
+ goto cleanup_clk;
+
+ /* Autodetect the need for 64-bit DMA pointers.
+ * When the IP is configured for a bus width bigger than 32 bits,
+ * writing the MSB registers is mandatory, even if they are all 0.
+ * We can detect this case by writing all 1's to one such register
+ * and see if that sticks: when the IP is configured for 32 bits
+ * only, those registers are RES0.
+ * Those MSB registers were introduced in IP v7.1, which we check first.
+ */
+ if ((axienet_ior(lp, XAE_ID_OFFSET) >> 24) >= 0x9) {
+ void __iomem *desc = lp->dma_regs + XAXIDMA_TX_CDESC_OFFSET + 4;
- /* Autodetect the need for 64-bit DMA pointers.
- * When the IP is configured for a bus width bigger than 32 bits,
- * writing the MSB registers is mandatory, even if they are all 0.
- * We can detect this case by writing all 1's to one such register
- * and see if that sticks: when the IP is configured for 32 bits
- * only, those registers are RES0.
- * Those MSB registers were introduced in IP v7.1, which we check first.
- */
- if ((axienet_ior(lp, XAE_ID_OFFSET) >> 24) >= 0x9) {
- void __iomem *desc = lp->dma_regs + XAXIDMA_TX_CDESC_OFFSET + 4;
-
- iowrite32(0x0, desc);
- if (ioread32(desc) == 0) { /* sanity check */
- iowrite32(0xffffffff, desc);
- if (ioread32(desc) > 0) {
- lp->features |= XAE_FEATURE_DMA_64BIT;
- addr_width = 64;
- dev_info(&pdev->dev,
- "autodetected 64-bit DMA range\n");
- }
iowrite32(0x0, desc);
+ if (ioread32(desc) == 0) { /* sanity check */
+ iowrite32(0xffffffff, desc);
+ if (ioread32(desc) > 0) {
+ lp->features |= XAE_FEATURE_DMA_64BIT;
+ addr_width = 64;
+ dev_info(&pdev->dev,
+ "autodetected 64-bit DMA range\n");
+ }
+ iowrite32(0x0, desc);
+ }
+ }
+ if (!IS_ENABLED(CONFIG_64BIT) && lp->features & XAE_FEATURE_DMA_64BIT) {
+ dev_err(&pdev->dev, "64-bit addressable DMA is not compatible with 32-bit archecture\n");
+ ret = -EINVAL;
+ goto cleanup_clk;
}
- }
- if (!IS_ENABLED(CONFIG_64BIT) && lp->features & XAE_FEATURE_DMA_64BIT) {
- dev_err(&pdev->dev, "64-bit addressable DMA is not compatible with 32-bit archecture\n");
- ret = -EINVAL;
- goto cleanup_clk;
- }
- ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(addr_width));
- if (ret) {
- dev_err(&pdev->dev, "No suitable DMA available\n");
- goto cleanup_clk;
+ ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(addr_width));
+ if (ret) {
+ dev_err(&pdev->dev, "No suitable DMA available\n");
+ goto cleanup_clk;
+ }
+ netif_napi_add(ndev, &lp->napi_rx, axienet_rx_poll);
+ netif_napi_add(ndev, &lp->napi_tx, axienet_tx_poll);
}
/* Check for Ethernet core IRQ (optional) */
@@ -2099,8 +2158,8 @@ static int axienet_probe(struct platform_device *pdev)
}
lp->coalesce_count_rx = XAXIDMA_DFT_RX_THRESHOLD;
- lp->coalesce_usec_rx = XAXIDMA_DFT_RX_USEC;
lp->coalesce_count_tx = XAXIDMA_DFT_TX_THRESHOLD;
+ lp->coalesce_usec_rx = XAXIDMA_DFT_RX_USEC;
lp->coalesce_usec_tx = XAXIDMA_DFT_TX_USEC;
ret = axienet_mdio_setup(lp);
--
2.34.1
On Tue, Sep 19, 2023 at 12:46:54AM +0530, Radhey Shyam Pandey wrote:
> +/**
> + * axienet_open - Driver open routine.
> + * @ndev: Pointer to net_device structure
> + *
> + * Return: 0, on success.
> + * non-zero error value on failure
> + *
> + * This is the driver open routine. It calls phylink_start to start the
> + * PHY device.
> + * It also allocates interrupt service routines, enables the interrupt lines
> + * and ISR handling. Axi Ethernet core is reset through Axi DMA core. Buffer
> + * descriptors are initialized.
> + */
> +static int axienet_open(struct net_device *ndev)
> +{
> + int ret;
> + struct axienet_local *lp = netdev_priv(ndev);
> +
> + dev_dbg(&ndev->dev, "%s\n", __func__);
> +
> + /* When we do an Axi Ethernet reset, it resets the complete core
> + * including the MDIO. MDIO must be disabled before resetting.
> + * Hold MDIO bus lock to avoid MDIO accesses during the reset.
> + */
> + axienet_lock_mii(lp);
> + ret = axienet_device_reset(ndev);
> + axienet_unlock_mii(lp);
> +
> + ret = phylink_of_phy_connect(lp->phylink, lp->dev->of_node, 0);
> + if (ret) {
> + dev_err(lp->dev, "phylink_of_phy_connect() failed: %d\n", ret);
> + return ret;
> + }
> +
> + phylink_start(lp->phylink);
... and at this point, the link can come up while phylink_start() is
completing. Could that cause a problem if this happens before:
> +
> + if (!lp->use_dmaengine) {
> + ret = axienet_init_legacy_dma(ndev);
> + if (ret)
> + goto error_code;
> + }
?
I suppose I should add this statement to the phylink_start()
documentation so that this point is clear for everyone.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> -----Original Message-----
> From: Russell King <[email protected]>
> Sent: Tuesday, September 19, 2023 3:22 PM
> To: Pandey, Radhey Shyam <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; Simek, Michal
> <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]; git (AMD-Xilinx) <[email protected]>; Sarath Babu
> Naidu Gaddam <[email protected]>
> Subject: Re: [PATCH net-next v6 2/3] net: axienet: Preparatory changes for
> dmaengine support
>
> On Tue, Sep 19, 2023 at 12:46:54AM +0530, Radhey Shyam Pandey wrote:
> > +/**
> > + * axienet_open - Driver open routine.
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * Return: 0, on success.
> > + * non-zero error value on failure
> > + *
> > + * This is the driver open routine. It calls phylink_start to start
> > +the
> > + * PHY device.
> > + * It also allocates interrupt service routines, enables the
> > +interrupt lines
> > + * and ISR handling. Axi Ethernet core is reset through Axi DMA core.
> > +Buffer
> > + * descriptors are initialized.
> > + */
> > +static int axienet_open(struct net_device *ndev) {
> > + int ret;
> > + struct axienet_local *lp = netdev_priv(ndev);
> > +
> > + dev_dbg(&ndev->dev, "%s\n", __func__);
> > +
> > + /* When we do an Axi Ethernet reset, it resets the complete core
> > + * including the MDIO. MDIO must be disabled before resetting.
> > + * Hold MDIO bus lock to avoid MDIO accesses during the reset.
> > + */
> > + axienet_lock_mii(lp);
> > + ret = axienet_device_reset(ndev);
> > + axienet_unlock_mii(lp);
> > +
> > + ret = phylink_of_phy_connect(lp->phylink, lp->dev->of_node, 0);
> > + if (ret) {
> > + dev_err(lp->dev, "phylink_of_phy_connect() failed: %d\n",
> ret);
> > + return ret;
> > + }
> > +
> > + phylink_start(lp->phylink);
>
> ... and at this point, the link can come up while phylink_start() is completing.
> Could that cause a problem if this happens before:
This preparatory patch keeps same execution sequence and it only
creates wrapper around dma specific initialization. No functional
change.
There shouldn't be any problem if link come up while phylink_start is
completing. Packet will be processed only after dma initialization
(Interrupts are enabled) .
>
> > +
> > + if (!lp->use_dmaengine) {
> > + ret = axienet_init_legacy_dma(ndev);
> > + if (ret)
> > + goto error_code;
> > + }
>
> ?
>
> I suppose I should add this statement to the phylink_start() documentation
> so that this point is clear for everyone.
Seems to be a good idea to capture it in documentation.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!