Re: [net-next PATCH 2/2] drivers: net: cpsw: add separate napi for tx packet handling for performance improvment
On Thursday 30 July 2015 04:27 AM, Francois Romieu wrote: Mugunthan V N mugunthan...@ti.com : On Wednesday 29 July 2015 04:00 AM, Francois Romieu wrote: Mugunthan V N mugunthan...@ti.com : On Tuesday 28 July 2015 02:52 AM, Francois Romieu wrote: [...] cpsw_ndo_stop calls napi_disable: you can remove netif_running. This netif_running check is to find which interface is up as the interrupt is shared by both the interfaces. When first interface is down and second interface is active then napi_schedule for first interface will fail and second interface napi needs to be scheduled. So I don't think netif_running needs to be removed. Each interface has its own napi tx (resp. rx) context: I would had expected two unconditional napi_schedule per tx (resp. rx) shared irq, not one. I'll read it again after some sleep. For each interrupt only one napi will be scheduled, when the first interface is down then only second interface napi is scheduled in both tx and rx irqs. Ok, I've had some hints from the Assumptions section at http://processors.wiki.ti.com/index.php/AM335x_CPSW_%28Ethernet%29_Driver%27s_Guide#Dual_Standalone_EMAC_mode Why does the driver create 2 rx napi contexts ? They don't run at the same time and the port demux is done in cpsw_dual_emac_src_port_detect. The driver would work the same with a single rx (resp. tx) napi context for both interfaces. The wiki you had pointed out is old design done on v3.2 and doesn't have device tree support also. In mainline Dual EMAC implementation has changed a lot. I can think of a way with one napi implementation for each rx and tx, will submit a separate patch for it. Regards Mugunthan V N -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [net-next PATCH 2/2] drivers: net: cpsw: add separate napi for tx packet handling for performance improvment
Mugunthan V N mugunthan...@ti.com : On Wednesday 29 July 2015 04:00 AM, Francois Romieu wrote: Mugunthan V N mugunthan...@ti.com : On Tuesday 28 July 2015 02:52 AM, Francois Romieu wrote: [...] cpsw_ndo_stop calls napi_disable: you can remove netif_running. This netif_running check is to find which interface is up as the interrupt is shared by both the interfaces. When first interface is down and second interface is active then napi_schedule for first interface will fail and second interface napi needs to be scheduled. So I don't think netif_running needs to be removed. Each interface has its own napi tx (resp. rx) context: I would had expected two unconditional napi_schedule per tx (resp. rx) shared irq, not one. I'll read it again after some sleep. For each interrupt only one napi will be scheduled, when the first interface is down then only second interface napi is scheduled in both tx and rx irqs. Ok, I've had some hints from the Assumptions section at http://processors.wiki.ti.com/index.php/AM335x_CPSW_%28Ethernet%29_Driver%27s_Guide#Dual_Standalone_EMAC_mode Why does the driver create 2 rx napi contexts ? They don't run at the same time and the port demux is done in cpsw_dual_emac_src_port_detect. The driver would work the same with a single rx (resp. tx) napi context for both interfaces. -- Ueimor -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [net-next PATCH 2/2] drivers: net: cpsw: add separate napi for tx packet handling for performance improvment
Mugunthan V N mugunthan...@ti.com : On Tuesday 28 July 2015 02:52 AM, Francois Romieu wrote: Mugunthan V N mugunthan...@ti.com : [...] @@ -752,13 +753,22 @@ static irqreturn_t cpsw_tx_interrupt(int irq, void *dev_id) struct cpsw_priv *priv = dev_id; cpdma_ctlr_eoi(priv-dma, CPDMA_EOI_TX); - cpdma_chan_process(priv-txch, 128); + writel(0, priv-wr_regs-tx_en); + + if (netif_running(priv-ndev)) { + napi_schedule(priv-napi_tx); + return IRQ_HANDLED; + } cpsw_ndo_stop calls napi_disable: you can remove netif_running. This netif_running check is to find which interface is up as the interrupt is shared by both the interfaces. When first interface is down and second interface is active then napi_schedule for first interface will fail and second interface napi needs to be scheduled. So I don't think netif_running needs to be removed. Each interface has its own napi tx (resp. rx) context: I would had expected two unconditional napi_schedule per tx (resp. rx) shared irq, not one. I'll read it again after some sleep. -- Ueimor -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [net-next PATCH 2/2] drivers: net: cpsw: add separate napi for tx packet handling for performance improvment
On Wednesday 29 July 2015 04:00 AM, Francois Romieu wrote: Mugunthan V N mugunthan...@ti.com : On Tuesday 28 July 2015 02:52 AM, Francois Romieu wrote: Mugunthan V N mugunthan...@ti.com : [...] @@ -752,13 +753,22 @@ static irqreturn_t cpsw_tx_interrupt(int irq, void *dev_id) struct cpsw_priv *priv = dev_id; cpdma_ctlr_eoi(priv-dma, CPDMA_EOI_TX); - cpdma_chan_process(priv-txch, 128); + writel(0, priv-wr_regs-tx_en); + + if (netif_running(priv-ndev)) { + napi_schedule(priv-napi_tx); + return IRQ_HANDLED; + } cpsw_ndo_stop calls napi_disable: you can remove netif_running. This netif_running check is to find which interface is up as the interrupt is shared by both the interfaces. When first interface is down and second interface is active then napi_schedule for first interface will fail and second interface napi needs to be scheduled. So I don't think netif_running needs to be removed. Each interface has its own napi tx (resp. rx) context: I would had expected two unconditional napi_schedule per tx (resp. rx) shared irq, not one. I'll read it again after some sleep. For each interrupt only one napi will be scheduled, when the first interface is down then only second interface napi is scheduled in both tx and rx irqs. Regards Mugunthan V N -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [net-next PATCH 2/2] drivers: net: cpsw: add separate napi for tx packet handling for performance improvment
On Tuesday 28 July 2015 02:52 AM, Francois Romieu wrote: Mugunthan V N mugunthan...@ti.com : [...] diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index d68d759..4f98537 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -752,13 +753,22 @@ static irqreturn_t cpsw_tx_interrupt(int irq, void *dev_id) struct cpsw_priv *priv = dev_id; cpdma_ctlr_eoi(priv-dma, CPDMA_EOI_TX); -cpdma_chan_process(priv-txch, 128); +writel(0, priv-wr_regs-tx_en); + +if (netif_running(priv-ndev)) { +napi_schedule(priv-napi_tx); +return IRQ_HANDLED; +} cpsw_ndo_stop calls napi_disable: you can remove netif_running. H, Will fix in v2. Regards Mugunthan V N -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [net-next PATCH 2/2] drivers: net: cpsw: add separate napi for tx packet handling for performance improvment
On Tuesday 28 July 2015 02:52 AM, Francois Romieu wrote: Mugunthan V N mugunthan...@ti.com : [...] diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index d68d759..4f98537 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -752,13 +753,22 @@ static irqreturn_t cpsw_tx_interrupt(int irq, void *dev_id) struct cpsw_priv *priv = dev_id; cpdma_ctlr_eoi(priv-dma, CPDMA_EOI_TX); -cpdma_chan_process(priv-txch, 128); +writel(0, priv-wr_regs-tx_en); + +if (netif_running(priv-ndev)) { +napi_schedule(priv-napi_tx); +return IRQ_HANDLED; +} cpsw_ndo_stop calls napi_disable: you can remove netif_running. This netif_running check is to find which interface is up as the interrupt is shared by both the interfaces. When first interface is down and second interface is active then napi_schedule for first interface will fail and second interface napi needs to be scheduled. So I don't think netif_running needs to be removed. Regards Mugunthan V N -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [net-next PATCH 2/2] drivers: net: cpsw: add separate napi for tx packet handling for performance improvment
Mugunthan V N mugunthan...@ti.com : [...] diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index d68d759..4f98537 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -752,13 +753,22 @@ static irqreturn_t cpsw_tx_interrupt(int irq, void *dev_id) struct cpsw_priv *priv = dev_id; cpdma_ctlr_eoi(priv-dma, CPDMA_EOI_TX); - cpdma_chan_process(priv-txch, 128); + writel(0, priv-wr_regs-tx_en); + + if (netif_running(priv-ndev)) { + napi_schedule(priv-napi_tx); + return IRQ_HANDLED; + } cpsw_ndo_stop calls napi_disable: you can remove netif_running. -- Ueimor -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html