[PATCH 2.6.32 3/3] net: Make ks8851 snl work under Beagle Zippy combo board

[PATCH 2.6.32 3/3] net: Make ks8851 snl work under Beagle Zippy combo board

[Ha, Tristram]

From: Tristram Ha <Tristram.Ha@micrel.com>

The Micrel KSZ8851 SNL Ethernet chip is used in the OMAP Beagle Zippy combo board.  Requesting interrupt using level triggering flag hangs the system because the interrupt handling routine is called continually.  Using edge triggering avoids this problem.  As a result, disabling interrupt may not be necessary.

Signed-off-by: Tristram Ha <Tristram.Ha@micrel.com>
---
diff -urpN linux-2.6.32.old/drivers/net/ks8851.c linux-2.6.32.new/drivers/net/ks8851.c
--- linux-2.6.32.old/drivers/net/ks8851.c	2009-11-03 11:37:49.000000000 -0800
+++ linux-2.6.32.new/drivers/net/ks8851.c	2009-12-02 15:31:39.000000000 -0800
@@ -398,7 +398,6 @@ static irqreturn_t ks8851_irq(int irq, v
 {
 	struct ks8851_net *ks = pw;
 
-	disable_irq_nosync(irq);
 	schedule_work(&ks->irq_work);
 	return IRQ_HANDLED;
 }
@@ -611,8 +610,6 @@ static void ks8851_irq_work(struct work_
 	}
 
 	mutex_unlock(&ks->lock);
-
-	enable_irq(ks->netdev->irq);
 }
 
 /**
@@ -1284,7 +1281,7 @@ static int __devinit ks8851_probe(struct
 	ks8851_init_mac(ks);
 	ks->tx_space = ks8851_rdreg16(ks, KS_TXMIR);
 
-	ret = request_irq(spi->irq, ks8851_irq, IRQF_TRIGGER_LOW,
+	ret = request_irq(spi->irq, ks8851_irq, IRQF_TRIGGER_FALLING,
 			  ndev->name, ks);
 	if (ret < 0) {
 		dev_err(&spi->dev, "failed to get irq\n");

Re: [PATCH 2.6.32 3/3] net: Make ks8851 snl work under Beagle Zippy combo board

[David Miller]

From: "Ha, Tristram" <Tristram.Ha@Micrel.Com>
Date: Thu, 3 Dec 2009 13:30:03 -0800

> From: Tristram Ha <Tristram.Ha@micrel.com>
> 
> The Micrel KSZ8851 SNL Ethernet chip is used in the OMAP Beagle Zippy combo board.  Requesting interrupt using level triggering flag hangs the system because the interrupt handling routine is called continually.  Using edge triggering avoids this problem.  As a result, disabling interrupt may not be necessary.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@micrel.com>

You say "may not be necessary", do you actually know for sure?

This seems questionable, if the interrupt disabling was there
for a reason what has changed to make it such that it is no
longer necessary?

RE: [PATCH 2.6.32 3/3] net: Make ks8851 snl work under Beagle Zippy combo board

[Ha, Tristram]

David Miller wrote:
> From: "Ha, Tristram" <Tristram.Ha@Micrel.Com>
> Date: Thu, 3 Dec 2009 13:30:03 -0800
> 
>> From: Tristram Ha <Tristram.Ha@micrel.com>
>> 
>> The Micrel KSZ8851 SNL Ethernet chip is used in the OMAP Beagle Zippy
combo board.  Requesting
>> interrupt using level triggering flag hangs the system because the
interrupt handling routine
>> is called continually.  Using edge triggering avoids this problem.
As a result, disabling
>> interrupt may not be necessary.   
>> 
>> Signed-off-by: Tristram Ha <Tristram.Ha@micrel.com>
> 
> You say "may not be necessary", do you actually know for sure?
> 
> This seems questionable, if the interrupt disabling was there for a
reason what has changed to
> make it such that it is no longer necessary? 

I have tested the KSZ8851 SNL driver using the OMAP Beagle Zippy combo
board, developed by Tin Can Tools.  Ben Dooks, who implemented the
original driver, has his own KSZ8851 SNL evaluation board, but he
probably did not run any performance test on the driver.  I did sent him
the changes before so that he can verify on his board, but I have not
received any response.

So I really do not know if interrupt disabling is necessary or not.  But
from my understanding of the SPI network driver model, it is almost
required the interrupt be edge triggered, as the hardware interrupt
cannot be disabled inside the actual interrupt handling routine.  So if
the interrupt is level triggered, the interrupt routine will be called
continually, and there is no chance for the workqueue to disable the
interrupt.  As the interrupt is edge triggered, I do not think the
interrupt needs to be disabled.

RE: [PATCH 2.6.32 3/3] net: Make ks8851 snl work under Beagle Zippy combo board

[Ha, Tristram]

Ha, Tristram wrote:
> David Miller wrote:
>> 
>> You say "may not be necessary", do you actually know for sure?
>> 
>> This seems questionable, if the interrupt disabling was there for a
>> reason what has changed to make it such that it is no longer
necessary?
> 
> I have tested the KSZ8851 SNL driver using the OMAP Beagle Zippy combo
board, developed by Tin
> Can Tools.  Ben Dooks, who implemented the original driver, has his
own KSZ8851 SNL evaluation
> board, but he probably did not run any performance test on the driver.
I did sent him the
> changes before so that he can verify on his board, but I have not
received any response.   
> 
> So I really do not know if interrupt disabling is necessary or not.
But from my understanding
> of the SPI network driver model, it is almost required the interrupt
be edge triggered, as the
> hardware interrupt cannot be disabled inside the actual interrupt
handling routine.  So if the
> interrupt is level triggered, the interrupt routine will be called
continually, and there is no
> chance for the workqueue to disable the interrupt.  As the interrupt
is edge triggered, I do not
> think the interrupt needs to be disabled.     

The "[PATCH 2.6.33] net: Fix ks8851 snl receive problem" I just
submitted turns off hardware interrupt during interrupt handling.  That
makes the disable_irq() call really unnecessary when the interrupt is
edge-triggered.  The code also works when the interrupt is
level-triggered under Beagle Zippy combo board, but theoretically the
disable_irq() is required when the interrupt is level-triggered.  When I
first worked on the ks8851 driver I was using Linux 2.6.22, and the
level-triggered interrupt code did not work.  That is why I changed the
code to edge-triggered.

Re: [PATCH 2.6.32 3/3] net: Make ks8851 snl work under Beagle Zippy combo board

[Ben Dooks]

Ha, Tristram wrote:
> From: Tristram Ha <Tristram.Ha@micrel.com>
> 
> The Micrel KSZ8851 SNL Ethernet chip is used in the OMAP Beagle Zippy combo board.  Requesting interrupt using level triggering flag hangs the system because the interrupt handling routine is called continually.  Using edge triggering avoids this problem.  As a result, disabling interrupt may not be necessary.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@micrel.com>

Making this sort of change could impact other users. I'll
submit a patch to get the interrupt type properly and push
everything else i've got,