[RFC,net-next,2/4] gianfar: Clear ievent from interrupt handler for [RT]x int

It's the interrupt handler's job to clear ievent for the Tx/Rx paths, as soon
as the corresponding interrupt sources have been masked.

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

[[RFC net-next 2/4] gianfar: Clear ievent from interrupt handler for [RT]x int] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:

> It's the interrupt handler's job to clear ievent for the Tx/Rx paths, as soon
> as the corresponding interrupt sources have been masked.

What wasn't clear to me was whether we'd ever have an instance of
gfar_poll run without RTX_MASK being cleared (in less normal conditions,
like netconsole, KGDBoE etc), since the gfar_schedule_cleanup is only
called from rx/tx IRQ threads, and neither of those are used by
gfar_poll, it seems.

Paul.
--
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
> ---
>  drivers/net/ethernet/freescale/gianfar.c |   16 ++++++----------
>  1 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
> index e9feeb9..ddd350a 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -2568,12 +2568,13 @@ static void gfar_schedule_cleanup(struct gfar_priv_grp *gfargrp)
>  	if (napi_schedule_prep(&gfargrp->napi)) {
>  		gfar_write(&gfargrp->regs->imask, IMASK_RTX_DISABLED);
>  		__napi_schedule(&gfargrp->napi);
> -	} else {
> -		/* Clear IEVENT, so interrupts aren't called again
> -		 * because of the packets that have already arrived.
> -		 */
> -		gfar_write(&gfargrp->regs->ievent, IEVENT_RTX_MASK);
>  	}
> +
> +	/* Clear IEVENT, so interrupts aren't called again
> +	 * because of the packets that have already arrived.
> +	 */
> +	gfar_write(&gfargrp->regs->ievent, IEVENT_RTX_MASK);
> +
>  	spin_unlock_irqrestore(&gfargrp->grplock, flags);
>  
>  }
> @@ -2837,11 +2838,6 @@ static int gfar_poll(struct napi_struct *napi, int budget)
>  	num_queues = gfargrp->num_rx_queues;
>  	budget_per_queue = budget/num_queues;
>  
> -	/* Clear IEVENT, so interrupts aren't called again
> -	 * because of the packets that have already arrived
> -	 */
> -	gfar_write(&regs->ievent, IEVENT_RTX_MASK);
> -
>  	while (num_queues && left_over_budget) {
>  		budget_per_queue = left_over_budget/num_queues;
>  		left_over_budget = 0;
> -- 
> 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

On 8/8/2012 7:11 PM, Paul Gortmaker wrote:
> [[RFC net-next 2/4] gianfar: Clear ievent from interrupt handler for [RT]x int] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:
>
>> It's the interrupt handler's job to clear ievent for the Tx/Rx paths, as soon
>> as the corresponding interrupt sources have been masked.
>
> What wasn't clear to me was whether we'd ever have an instance of
> gfar_poll run without RTX_MASK being cleared (in less normal conditions,
> like netconsole, KGDBoE etc), since the gfar_schedule_cleanup is only
> called from rx/tx IRQ threads, and neither of those are used by
> gfar_poll, it seems.

Hi Paul,

As I see it, netconsole has the ndo_poll_controller hook, which points
to gfar_netpoll() -> gfar_interrupt() -> gfar_receive|transmit() ->
gfar_schedule_cleanup(), so it passes through schedule_cleanup.

My understanding is that gfar_poll() is scheduled for execution
only by "__napi_schedule(&gfargrp->napi);" from the Rx/Tx interrupt
handler (gfar_receive/transmit()->gfar_schedule_cleanup()), and that
it will be executed sometimes after the interrupt handler returns,
which means RTX_MASK has been cleared (and the interrupt sources are
already masked).

I think that we might have an issue if we don't clear IEVENT right away
in the interrupt handler, as this register might cause additional hw
interrupts to the PIC upon returning from the interrupt handler.


--
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