diff mbox

[net-next,07/16] sfc: Limit scope of a Falcon A1 IRQ workaround

Message ID 1377471823.2586.76.camel@deadeye.wl.decadent.org.uk
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Ben Hutchings Aug. 25, 2013, 11:03 p.m. UTC
We unconditionally acknowledge legacy interrupts just before disabling
them.  This workaround is needed on Falcon A1 but probably not on
later chips where the legacy interrupt mechanism is different.  It was
also originally done after the IRQ handler was removed, not before.
Restore the original behaviour for Falcon A1 only by doing this
acknowledgement in the efx_nic_type::fini operation.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/falcon.c |    4 ++--
 drivers/net/ethernet/sfc/nic.c    |    7 -------
 drivers/net/ethernet/sfc/nic.h    |    1 -
 3 files changed, 2 insertions(+), 10 deletions(-)

Comments

Sergei Shtylyov Aug. 26, 2013, 12:31 p.m. UTC | #1
Hello.

On 26-08-2013 3:03, Ben Hutchings wrote:

> We unconditionally acknowledge legacy interrupts just before disabling
> them.  This workaround is needed on Falcon A1 but probably not on
> later chips where the legacy interrupt mechanism is different.  It was
> also originally done after the IRQ handler was removed, not before.
> Restore the original behaviour for Falcon A1 only by doing this
> acknowledgement in the efx_nic_type::fini operation.

> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
>   drivers/net/ethernet/sfc/falcon.c |    4 ++--
>   drivers/net/ethernet/sfc/nic.c    |    7 -------
>   drivers/net/ethernet/sfc/nic.h    |    1 -
>   3 files changed, 2 insertions(+), 10 deletions(-)

> diff --git a/drivers/net/ethernet/sfc/falcon.c b/drivers/net/ethernet/sfc/falcon.c
> index f8de382..4492129 100644
> --- a/drivers/net/ethernet/sfc/falcon.c
> +++ b/drivers/net/ethernet/sfc/falcon.c
> @@ -336,7 +336,7 @@ static void falcon_prepare_flush(struct efx_nic *efx)
>    *
>    * NB most hardware supports MSI interrupts
>    */
> -inline void falcon_irq_ack_a1(struct efx_nic *efx)
> +static inline void falcon_irq_ack_a1(struct efx_nic *efx)

    Does inline make sense still when this now is referenced indirectly?

>   {
>   	efx_dword_t reg;
>
> @@ -2343,7 +2343,7 @@ const struct efx_nic_type falcon_a1_nic_type = {
>   	.remove = falcon_remove_nic,
>   	.init = falcon_init_nic,
>   	.dimension_resources = falcon_dimension_resources,
> -	.fini = efx_port_dummy_op_void,
> +	.fini = falcon_irq_ack_a1,
>   	.monitor = falcon_monitor,
>   	.map_reset_reason = falcon_map_reset_reason,
>   	.map_reset_flags = falcon_map_reset_flags,

WBR, Sergei

--
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
Ben Hutchings Aug. 26, 2013, 10:47 p.m. UTC | #2
On Mon, 2013-08-26 at 16:31 +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 26-08-2013 3:03, Ben Hutchings wrote:
> 
> > We unconditionally acknowledge legacy interrupts just before disabling
> > them.  This workaround is needed on Falcon A1 but probably not on
> > later chips where the legacy interrupt mechanism is different.  It was
> > also originally done after the IRQ handler was removed, not before.
> > Restore the original behaviour for Falcon A1 only by doing this
> > acknowledgement in the efx_nic_type::fini operation.
> 
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
> >   drivers/net/ethernet/sfc/falcon.c |    4 ++--
> >   drivers/net/ethernet/sfc/nic.c    |    7 -------
> >   drivers/net/ethernet/sfc/nic.h    |    1 -
> >   3 files changed, 2 insertions(+), 10 deletions(-)
> 
> > diff --git a/drivers/net/ethernet/sfc/falcon.c b/drivers/net/ethernet/sfc/falcon.c
> > index f8de382..4492129 100644
> > --- a/drivers/net/ethernet/sfc/falcon.c
> > +++ b/drivers/net/ethernet/sfc/falcon.c
> > @@ -336,7 +336,7 @@ static void falcon_prepare_flush(struct efx_nic *efx)
> >    *
> >    * NB most hardware supports MSI interrupts
> >    */
> > -inline void falcon_irq_ack_a1(struct efx_nic *efx)
> > +static inline void falcon_irq_ack_a1(struct efx_nic *efx)
> 
>     Does inline make sense still when this now is referenced indirectly?

It's not inlined in the cleanup code, either before or after this
change.  But we do want this to be inlined in the following function,
falcon_legacy_interrupt_a1().  So I think it does make sense.

Ben.

> >   {
> >   	efx_dword_t reg;
> >
> > @@ -2343,7 +2343,7 @@ const struct efx_nic_type falcon_a1_nic_type = {
> >   	.remove = falcon_remove_nic,
> >   	.init = falcon_init_nic,
> >   	.dimension_resources = falcon_dimension_resources,
> > -	.fini = efx_port_dummy_op_void,
> > +	.fini = falcon_irq_ack_a1,
> >   	.monitor = falcon_monitor,
> >   	.map_reset_reason = falcon_map_reset_reason,
> >   	.map_reset_flags = falcon_map_reset_flags,
> 
> WBR, Sergei
>
diff mbox

Patch

diff --git a/drivers/net/ethernet/sfc/falcon.c b/drivers/net/ethernet/sfc/falcon.c
index f8de382..4492129 100644
--- a/drivers/net/ethernet/sfc/falcon.c
+++ b/drivers/net/ethernet/sfc/falcon.c
@@ -336,7 +336,7 @@  static void falcon_prepare_flush(struct efx_nic *efx)
  *
  * NB most hardware supports MSI interrupts
  */
-inline void falcon_irq_ack_a1(struct efx_nic *efx)
+static inline void falcon_irq_ack_a1(struct efx_nic *efx)
 {
 	efx_dword_t reg;
 
@@ -2343,7 +2343,7 @@  const struct efx_nic_type falcon_a1_nic_type = {
 	.remove = falcon_remove_nic,
 	.init = falcon_init_nic,
 	.dimension_resources = falcon_dimension_resources,
-	.fini = efx_port_dummy_op_void,
+	.fini = falcon_irq_ack_a1,
 	.monitor = falcon_monitor,
 	.map_reset_reason = falcon_map_reset_reason,
 	.map_reset_flags = falcon_map_reset_flags,
diff --git a/drivers/net/ethernet/sfc/nic.c b/drivers/net/ethernet/sfc/nic.c
index c17d659..2d0a584 100644
--- a/drivers/net/ethernet/sfc/nic.c
+++ b/drivers/net/ethernet/sfc/nic.c
@@ -1781,7 +1781,6 @@  int efx_nic_init_interrupt(struct efx_nic *efx)
 void efx_nic_fini_interrupt(struct efx_nic *efx)
 {
 	struct efx_channel *channel;
-	efx_oword_t reg;
 
 #ifdef CONFIG_RFS_ACCEL
 	free_irq_cpu_rmap(efx->net_dev->rx_cpu_rmap);
@@ -1792,12 +1791,6 @@  void efx_nic_fini_interrupt(struct efx_nic *efx)
 	efx_for_each_channel(channel, efx)
 		free_irq(channel->irq, &efx->msi_context[channel->channel]);
 
-	/* ACK legacy interrupt */
-	if (efx_nic_rev(efx) >= EFX_REV_FALCON_B0)
-		efx_reado(efx, &reg, FR_BZ_INT_ISR0);
-	else
-		falcon_irq_ack_a1(efx);
-
 	/* Disable legacy interrupt */
 	if (efx->legacy_irq)
 		free_irq(efx->legacy_irq, efx);
diff --git a/drivers/net/ethernet/sfc/nic.h b/drivers/net/ethernet/sfc/nic.h
index 9120e8b..33aa120 100644
--- a/drivers/net/ethernet/sfc/nic.h
+++ b/drivers/net/ethernet/sfc/nic.h
@@ -308,7 +308,6 @@  extern void efx_nic_disable_interrupts(struct efx_nic *efx);
 extern void efx_nic_fini_interrupt(struct efx_nic *efx);
 extern irqreturn_t efx_nic_fatal_interrupt(struct efx_nic *efx);
 extern irqreturn_t falcon_legacy_interrupt_a1(int irq, void *dev_id);
-extern void falcon_irq_ack_a1(struct efx_nic *efx);
 
 static inline int efx_nic_event_test_irq_cpu(struct efx_channel *channel)
 {