Re: [net-next PATCH 2/2] drivers: net: cpsw: add separate napi for tx packet handling for performance improvment

2015-07-30 Thread Mugunthan V N
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

2015-07-29 Thread Francois Romieu
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

2015-07-28 Thread Francois Romieu
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

2015-07-28 Thread Mugunthan V N
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

2015-07-28 Thread Mugunthan V N
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

2015-07-28 Thread Mugunthan V N
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

2015-07-27 Thread Francois Romieu
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