diff mbox

[RFC,net-next,3/4] gianfar: Separate out the Rx and Tx coalescing functions

Message ID 1344428810-29923-4-git-send-email-claudiu.manoil@freescale.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Claudiu Manoil Aug. 8, 2012, 12:26 p.m. UTC
Split the coalescing programming support by Rx and Tx h/w queues, in order to
introduce a separate NAPI for the Tx confirmation path (next patch). This way,
the Rx processing path will handle the coalescing settings for the Rx queues
only, resp. the Tx confirmation processing path will handle the Tx queues.

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/ethernet/freescale/gianfar.c |   36 +++++++++++++++++++++++------
 1 files changed, 28 insertions(+), 8 deletions(-)

Comments

Paul Gortmaker Aug. 8, 2012, 3:44 p.m. UTC | #1
[[RFC net-next 3/4] gianfar: Separate out the Rx and Tx coalescing functions] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:

> Split the coalescing programming support by Rx and Tx h/w queues, in order to
> introduce a separate NAPI for the Tx confirmation path (next patch). This way,
> the Rx processing path will handle the coalescing settings for the Rx queues
> only, resp. the Tx confirmation processing path will handle the Tx queues.
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
> ---
>  drivers/net/ethernet/freescale/gianfar.c |   36 +++++++++++++++++++++++------
>  1 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
> index ddd350a..919acb3 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -1794,8 +1794,8 @@ void gfar_start(struct net_device *dev)
>  	dev->trans_start = jiffies; /* prevent tx timeout */
>  }
>  
> -void gfar_configure_coalescing(struct gfar_private *priv,
> -			       unsigned long tx_mask, unsigned long rx_mask)
> +static inline void gfar_configure_tx_coalescing(struct gfar_private *priv,

I believe the preference is to not specify inline when all the chunks in
play are present in the one C file -- i.e. let gcc figure it out.  Same
for the Rx instance below.

P.
--

> +						unsigned long mask)
>  {
>  	struct gfar __iomem *regs = priv->gfargrp[0].regs;
>  	u32 __iomem *baddr;
> @@ -1803,14 +1803,31 @@ void gfar_configure_coalescing(struct gfar_private *priv,
>  	if (priv->mode == MQ_MG_MODE) {
>  		int i;
>  		baddr = &regs->txic0;
> -		for_each_set_bit(i, &tx_mask, priv->num_tx_queues) {
> +		for_each_set_bit(i, &mask, priv->num_tx_queues) {
>  			gfar_write(baddr + i, 0);
>  			if (likely(priv->tx_queue[i]->txcoalescing))
>  				gfar_write(baddr + i, priv->tx_queue[i]->txic);
>  		}
> +	} else {
> +		/* Backward compatible case ---- even if we enable
> +		 * multiple queues, there's only single reg to program
> +		 */
> +		gfar_write(&regs->txic, 0);
> +		if (likely(priv->tx_queue[0]->txcoalescing))
> +			gfar_write(&regs->txic, priv->tx_queue[0]->txic);
> +	}
> +}
> +
> +static inline void gfar_configure_rx_coalescing(struct gfar_private *priv,
> +						unsigned long mask)
> +{
> +	struct gfar __iomem *regs = priv->gfargrp[0].regs;
> +	u32 __iomem *baddr;
>  
> +	if (priv->mode == MQ_MG_MODE) {
> +		int i;
>  		baddr = &regs->rxic0;
> -		for_each_set_bit(i, &rx_mask, priv->num_rx_queues) {
> +		for_each_set_bit(i, &mask, priv->num_rx_queues) {
>  			gfar_write(baddr + i, 0);
>  			if (likely(priv->rx_queue[i]->rxcoalescing))
>  				gfar_write(baddr + i, priv->rx_queue[i]->rxic);
> @@ -1819,16 +1836,19 @@ void gfar_configure_coalescing(struct gfar_private *priv,
>  		/* Backward compatible case ---- even if we enable
>  		 * multiple queues, there's only single reg to program
>  		 */
> -		gfar_write(&regs->txic, 0);
> -		if (likely(priv->tx_queue[0]->txcoalescing))
> -			gfar_write(&regs->txic, priv->tx_queue[0]->txic);
> -
>  		gfar_write(&regs->rxic, 0);
>  		if (likely(priv->rx_queue[0]->rxcoalescing))
>  			gfar_write(&regs->rxic, priv->rx_queue[0]->rxic);
>  	}
>  }
>  
> +void gfar_configure_coalescing(struct gfar_private *priv,
> +			       unsigned long tx_mask, unsigned long rx_mask)
> +{
> +	gfar_configure_tx_coalescing(priv, tx_mask);
> +	gfar_configure_rx_coalescing(priv, rx_mask);
> +}
> +
>  static int register_grp_irqs(struct gfar_priv_grp *grp)
>  {
>  	struct gfar_private *priv = grp->priv;
> -- 
> 1.6.6
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Claudiu Manoil Aug. 9, 2012, 4:24 p.m. UTC | #2
On 8/8/2012 6:44 PM, Paul Gortmaker wrote:
> [[RFC net-next 3/4] gianfar: Separate out the Rx and Tx coalescing functions] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:
>
>> Split the coalescing programming support by Rx and Tx h/w queues, in order to
>> introduce a separate NAPI for the Tx confirmation path (next patch). This way,
>> the Rx processing path will handle the coalescing settings for the Rx queues
>> only, resp. the Tx confirmation processing path will handle the Tx queues.
>>
>> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
>> ---
>>   drivers/net/ethernet/freescale/gianfar.c |   36 +++++++++++++++++++++++------
>>   1 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
>> index ddd350a..919acb3 100644
>> --- a/drivers/net/ethernet/freescale/gianfar.c
>> +++ b/drivers/net/ethernet/freescale/gianfar.c
>> @@ -1794,8 +1794,8 @@ void gfar_start(struct net_device *dev)
>>   	dev->trans_start = jiffies; /* prevent tx timeout */
>>   }
>>
>> -void gfar_configure_coalescing(struct gfar_private *priv,
>> -			       unsigned long tx_mask, unsigned long rx_mask)
>> +static inline void gfar_configure_tx_coalescing(struct gfar_private *priv,
>
> I believe the preference is to not specify inline when all the chunks in
> play are present in the one C file -- i.e. let gcc figure it out.  Same
> for the Rx instance below.
>
> P.
> --
I agree with you.
thanks


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Gortmaker Aug. 15, 2012, 1:29 a.m. UTC | #3
[[RFC net-next 3/4] gianfar: Separate out the Rx and Tx coalescing functions] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:

> Split the coalescing programming support by Rx and Tx h/w queues, in order to
> introduce a separate NAPI for the Tx confirmation path (next patch). This way,
> the Rx processing path will handle the coalescing settings for the Rx queues
> only, resp. the Tx confirmation processing path will handle the Tx queues.
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
> ---
>  drivers/net/ethernet/freescale/gianfar.c |   36 +++++++++++++++++++++++------
>  1 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
> index ddd350a..919acb3 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -1794,8 +1794,8 @@ void gfar_start(struct net_device *dev)
>  	dev->trans_start = jiffies; /* prevent tx timeout */
>  }
>  
> -void gfar_configure_coalescing(struct gfar_private *priv,
> -			       unsigned long tx_mask, unsigned long rx_mask)
> +static inline void gfar_configure_tx_coalescing(struct gfar_private *priv,
> +						unsigned long mask)

Hi Claudiu,

I had it in mind to mention this earlier, but forgot.  Align line two
(and three, and four...) of args with the 1st arg in line one, and
you'll save yourself needing a resend for basic formatting issues.

In other words, you need:

void foo(int some_really_long_line, struct blah *more_long_line,
         int b, ... )

i.e. allowing for mail mangling, make sure you have the the two int
directly over each other, in the example above.

Thanks,
Paul.
--

>  {
>  	struct gfar __iomem *regs = priv->gfargrp[0].regs;
>  	u32 __iomem *baddr;
> @@ -1803,14 +1803,31 @@ void gfar_configure_coalescing(struct gfar_private *priv,
>  	if (priv->mode == MQ_MG_MODE) {
>  		int i;
>  		baddr = &regs->txic0;
> -		for_each_set_bit(i, &tx_mask, priv->num_tx_queues) {
> +		for_each_set_bit(i, &mask, priv->num_tx_queues) {
>  			gfar_write(baddr + i, 0);
>  			if (likely(priv->tx_queue[i]->txcoalescing))
>  				gfar_write(baddr + i, priv->tx_queue[i]->txic);
>  		}
> +	} else {
> +		/* Backward compatible case ---- even if we enable
> +		 * multiple queues, there's only single reg to program
> +		 */
> +		gfar_write(&regs->txic, 0);
> +		if (likely(priv->tx_queue[0]->txcoalescing))
> +			gfar_write(&regs->txic, priv->tx_queue[0]->txic);
> +	}
> +}
> +
> +static inline void gfar_configure_rx_coalescing(struct gfar_private *priv,
> +						unsigned long mask)
> +{
> +	struct gfar __iomem *regs = priv->gfargrp[0].regs;
> +	u32 __iomem *baddr;
>  
> +	if (priv->mode == MQ_MG_MODE) {
> +		int i;
>  		baddr = &regs->rxic0;
> -		for_each_set_bit(i, &rx_mask, priv->num_rx_queues) {
> +		for_each_set_bit(i, &mask, priv->num_rx_queues) {
>  			gfar_write(baddr + i, 0);
>  			if (likely(priv->rx_queue[i]->rxcoalescing))
>  				gfar_write(baddr + i, priv->rx_queue[i]->rxic);
> @@ -1819,16 +1836,19 @@ void gfar_configure_coalescing(struct gfar_private *priv,
>  		/* Backward compatible case ---- even if we enable
>  		 * multiple queues, there's only single reg to program
>  		 */
> -		gfar_write(&regs->txic, 0);
> -		if (likely(priv->tx_queue[0]->txcoalescing))
> -			gfar_write(&regs->txic, priv->tx_queue[0]->txic);
> -
>  		gfar_write(&regs->rxic, 0);
>  		if (likely(priv->rx_queue[0]->rxcoalescing))
>  			gfar_write(&regs->rxic, priv->rx_queue[0]->rxic);
>  	}
>  }
>  
> +void gfar_configure_coalescing(struct gfar_private *priv,
> +			       unsigned long tx_mask, unsigned long rx_mask)
> +{
> +	gfar_configure_tx_coalescing(priv, tx_mask);
> +	gfar_configure_rx_coalescing(priv, rx_mask);
> +}
> +
>  static int register_grp_irqs(struct gfar_priv_grp *grp)
>  {
>  	struct gfar_private *priv = grp->priv;
> -- 
> 1.6.6
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index ddd350a..919acb3 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -1794,8 +1794,8 @@  void gfar_start(struct net_device *dev)
 	dev->trans_start = jiffies; /* prevent tx timeout */
 }
 
-void gfar_configure_coalescing(struct gfar_private *priv,
-			       unsigned long tx_mask, unsigned long rx_mask)
+static inline void gfar_configure_tx_coalescing(struct gfar_private *priv,
+						unsigned long mask)
 {
 	struct gfar __iomem *regs = priv->gfargrp[0].regs;
 	u32 __iomem *baddr;
@@ -1803,14 +1803,31 @@  void gfar_configure_coalescing(struct gfar_private *priv,
 	if (priv->mode == MQ_MG_MODE) {
 		int i;
 		baddr = &regs->txic0;
-		for_each_set_bit(i, &tx_mask, priv->num_tx_queues) {
+		for_each_set_bit(i, &mask, priv->num_tx_queues) {
 			gfar_write(baddr + i, 0);
 			if (likely(priv->tx_queue[i]->txcoalescing))
 				gfar_write(baddr + i, priv->tx_queue[i]->txic);
 		}
+	} else {
+		/* Backward compatible case ---- even if we enable
+		 * multiple queues, there's only single reg to program
+		 */
+		gfar_write(&regs->txic, 0);
+		if (likely(priv->tx_queue[0]->txcoalescing))
+			gfar_write(&regs->txic, priv->tx_queue[0]->txic);
+	}
+}
+
+static inline void gfar_configure_rx_coalescing(struct gfar_private *priv,
+						unsigned long mask)
+{
+	struct gfar __iomem *regs = priv->gfargrp[0].regs;
+	u32 __iomem *baddr;
 
+	if (priv->mode == MQ_MG_MODE) {
+		int i;
 		baddr = &regs->rxic0;
-		for_each_set_bit(i, &rx_mask, priv->num_rx_queues) {
+		for_each_set_bit(i, &mask, priv->num_rx_queues) {
 			gfar_write(baddr + i, 0);
 			if (likely(priv->rx_queue[i]->rxcoalescing))
 				gfar_write(baddr + i, priv->rx_queue[i]->rxic);
@@ -1819,16 +1836,19 @@  void gfar_configure_coalescing(struct gfar_private *priv,
 		/* Backward compatible case ---- even if we enable
 		 * multiple queues, there's only single reg to program
 		 */
-		gfar_write(&regs->txic, 0);
-		if (likely(priv->tx_queue[0]->txcoalescing))
-			gfar_write(&regs->txic, priv->tx_queue[0]->txic);
-
 		gfar_write(&regs->rxic, 0);
 		if (likely(priv->rx_queue[0]->rxcoalescing))
 			gfar_write(&regs->rxic, priv->rx_queue[0]->rxic);
 	}
 }
 
+void gfar_configure_coalescing(struct gfar_private *priv,
+			       unsigned long tx_mask, unsigned long rx_mask)
+{
+	gfar_configure_tx_coalescing(priv, tx_mask);
+	gfar_configure_rx_coalescing(priv, rx_mask);
+}
+
 static int register_grp_irqs(struct gfar_priv_grp *grp)
 {
 	struct gfar_private *priv = grp->priv;