diff mbox

cpsw: Fix interrupt storm among other things

Message ID 1359378668-26536-1-git-send-email-panto@antoniou-consulting.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pantelis Antoniou Jan. 28, 2013, 1:11 p.m. UTC
Fix interrupt storm on bone A4 cause by non-by-the-book interrupt handling.
While at it, added a non-NAPI mode (which is easier to debug), plus
some general fixes.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt |   1 +
 drivers/net/ethernet/ti/cpsw.c                 | 222 +++++++++++++++++++++----
 drivers/net/ethernet/ti/davinci_cpdma.c        |   4 +-
 drivers/net/ethernet/ti/davinci_cpdma.h        |   2 +-
 include/linux/platform_data/cpsw.h             |   1 +
 5 files changed, 194 insertions(+), 36 deletions(-)

Comments

Richard Cochran Jan. 28, 2013, 6:24 p.m. UTC | #1
On Mon, Jan 28, 2013 at 03:11:08PM +0200, Pantelis Antoniou wrote:
> Fix interrupt storm on bone A4 cause by non-by-the-book interrupt handling.
> While at it, added a non-NAPI mode (which is easier to debug), plus
> some general fixes.

I have a few issues with this patch:

1. This is a networking patch. It should be addressed to netdev, it it
   needs to have davem on CC.

2. The description is poor. You need to tell us more about this
   "storm". How can one trigger it? What is the effect? Does the
   system lock up, or is the throughput poor? Tell us exactly what the
   problem is. Tell us what is wrong in the interrupt handling, and
   how the patch improves the situation.

3. Don't just say "general fixes", but do say exactly what you fixed.

4. Adding non-NAPI code is going backwards. Don't do that (and see the
   recent discussion on netdev on just this very topic: Frank Li and
   the fec driver).

> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 40aff68..b6ca4af 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -148,10 +148,37 @@ struct cpsw_wr_regs {
>  	u32	soft_reset;
>  	u32	control;
>  	u32	int_control;
> -	u32	rx_thresh_en;
> -	u32	rx_en;
> -	u32	tx_en;
> -	u32	misc_en;
> +	u32	c0_rx_thresh_en;
> +	u32	c0_rx_en;
> +	u32	c0_tx_en;
> +	u32	c0_misc_en;

How does renaming these help?

(If you really think that new names are needed, then put the cosmetic
renaming changes into its a separate patch.)

> +	u32	c1_rx_thresh_en;
> +	u32	c1_rx_en;
> +	u32	c1_tx_en;
> +	u32	c1_misc_en;

You added a bunch of new fields, but you don't use any of them.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Cochran Jan. 28, 2013, 6:27 p.m. UTC | #2
On Mon, Jan 28, 2013 at 03:11:08PM +0200, Pantelis Antoniou wrote:
> Fix interrupt storm on bone A4 cause by non-by-the-book interrupt handling.
> While at it, added a non-NAPI mode (which is easier to debug), plus
> some general fixes.

This patch does not apply to net-next.

When you do post to netdev, please also put "net" or "net-next" into
the subject line.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pantelis Antoniou Jan. 28, 2013, 6:40 p.m. UTC | #3
Hi Richard,

Yes, I guess this was more of a drive-by patch dump - but people need this
to get PG2.0 silicon to work on am33xx.
 
On Jan 28, 2013, at 8:24 PM, Richard Cochran wrote:

> On Mon, Jan 28, 2013 at 03:11:08PM +0200, Pantelis Antoniou wrote:
>> Fix interrupt storm on bone A4 cause by non-by-the-book interrupt handling.
>> While at it, added a non-NAPI mode (which is easier to debug), plus
>> some general fixes.
> 
> I have a few issues with this patch:
> 
> 1. This is a networking patch. It should be addressed to netdev, it it
>   needs to have davem on CC.
> 
> 2. The description is poor. You need to tell us more about this
>   "storm". How can one trigger it? What is the effect? Does the
>   system lock up, or is the throughput poor? Tell us exactly what the
>   problem is. Tell us what is wrong in the interrupt handling, and
>   how the patch improves the situation.
> 

PG2.0 fixed a number of silicon bugs. The old driver hard locked, since
it didn't follow the TRM described interrupt handling.

> 3. Don't just say "general fixes", but do say exactly what you fixed.
> 
> 4. Adding non-NAPI code is going backwards. Don't do that (and see the
>   recent discussion on netdev on just this very topic: Frank Li and
>   the fec driver).
> 

Speaking of which, I'm probably the original developer of the fec driver.

And no, I don't think having a non-NAPI code path is backwards, especially
when trying to debug hardware problems; the non-NAPI driver is much easier
to understand and follow, especially when there is a convoluted method
you have to follow to have the h/w acknowledge the interrupt.
Not every device can be conveniently be made to shut up so that NAPI processing 
can take place at a later time.

The NAPI case is still broken in this driver, which OOPs under load.

>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
>> index 40aff68..b6ca4af 100644
>> --- a/drivers/net/ethernet/ti/cpsw.c
>> +++ b/drivers/net/ethernet/ti/cpsw.c
>> @@ -148,10 +148,37 @@ struct cpsw_wr_regs {
>> 	u32	soft_reset;
>> 	u32	control;
>> 	u32	int_control;
>> -	u32	rx_thresh_en;
>> -	u32	rx_en;
>> -	u32	tx_en;
>> -	u32	misc_en;
>> +	u32	c0_rx_thresh_en;
>> +	u32	c0_rx_en;
>> +	u32	c0_tx_en;
>> +	u32	c0_misc_en;
> 
> How does renaming these help?
> 
> (If you really think that new names are needed, then put the cosmetic
> renaming changes into its a separate patch.)

Those are the real register names in the updated TRM.

> 
>> +	u32	c1_rx_thresh_en;
>> +	u32	c1_rx_en;
>> +	u32	c1_tx_en;
>> +	u32	c1_misc_en;
> 
> You added a bunch of new fields, but you don't use any of them.
> 

dido.

> Thanks,
> Richard

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Cochran Jan. 28, 2013, 7:49 p.m. UTC | #4
On Mon, Jan 28, 2013 at 08:40:25PM +0200, Pantelis Antoniou wrote:
> Hi Richard,
> 
> Yes, I guess this was more of a drive-by patch dump - but people need this
> to get PG2.0 silicon to work on am33xx.

And what is PG2.0?

> And no, I don't think having a non-NAPI code path is backwards, especially
> when trying to debug hardware problems; the non-NAPI driver is much easier
> to understand and follow, especially when there is a convoluted method
> you have to follow to have the h/w acknowledge the interrupt.

Special debug modes are fine to have, but not in mainline code. 
I suggest taking a look at the recent netdev discussion on this.
Someone wanted to make napi/non-napi a DT option, and it got
nixed.

> Not every device can be conveniently be made to shut up so that NAPI processing 
> can take place at a later time.

So, are you saying that the cpsw cannot work as a napi device?
 
> The NAPI case is still broken in this driver, which OOPs under load.

And does your patch fix it, or not?

It would be nice to know what the problem is, and how to reproduce
it. I haven't seen any OOPs on the beaglebone, but maybe I am not
trying hard enough.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Cochran Jan. 28, 2013, 7:56 p.m. UTC | #5
On Mon, Jan 28, 2013 at 08:40:25PM +0200, Pantelis Antoniou wrote:
> 
> Speaking of which, I'm probably the original developer of the fec driver.

BTW, as I mentioned, someone is converting fec to napi. Care to take a
look to make sure it is done right?

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Korsgaard Jan. 28, 2013, 9:14 p.m. UTC | #6
>>>>> "Pantelis" == Pantelis Antoniou <panto@antoniou-consulting.com> writes:

'among other things' is not a very descriptive commit message.

 Pantelis> Fix interrupt storm on bone A4 cause by non-by-the-book
 Pantelis> interrupt handling.  While at it, added a non-NAPI mode
 Pantelis> (which is easier to debug), plus some general fixes.

'bone A4' is also not very descriptive. There also was an A4 revision of
the "old" beaglebone. I guess you're instead referring to a new die
revision?

What is the impact of this change on earlier devices?

 Pantelis> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
 Pantelis> @@ -20,6 +20,7 @@ Required properties:
 Pantelis>  - cpts_clock_shift	: Denominator to convert input clock ticks into nanoseconds
 Pantelis>  - phy_id		: Specifies slave phy id
 Pantelis>  - mac-address		: Specifies slave MAC address
 Pantelis> +- disable-napi		: Disables driver NAPI

napi is not a hardware feature, so it doesn't belong here (if anything,
it should be linux,disable-napi).
Mugunthan V N Jan. 29, 2013, 11:45 a.m. UTC | #7
On 1/28/2013 6:41 PM, Pantelis Antoniou wrote:
> Fix interrupt storm on bone A4 cause by non-by-the-book interrupt handling.
> While at it, added a non-NAPI mode (which is easier to debug), plus
> some general fixes.
>
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>   Documentation/devicetree/bindings/net/cpsw.txt |   1 +
>   drivers/net/ethernet/ti/cpsw.c                 | 222 +++++++++++++++++++++----
>   drivers/net/ethernet/ti/davinci_cpdma.c        |   4 +-
>   drivers/net/ethernet/ti/davinci_cpdma.h        |   2 +-
>   include/linux/platform_data/cpsw.h             |   1 +
>   5 files changed, 194 insertions(+), 36 deletions(-)
I have tested CPSW on AM335x EVM 1.5A with flood ping and i am not
seeing any interrupt storm.
Can you provide more details on how to reproduce the issue.

Regards
Mugunthan V N
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pantelis Antoniou Jan. 30, 2013, 8:34 a.m. UTC | #8
Hi Peter,

On Jan 28, 2013, at 11:14 PM, Peter Korsgaard wrote:

>>>>>> "Pantelis" == Pantelis Antoniou <panto@antoniou-consulting.com> writes:
> 
> 'among other things' is not a very descriptive commit message.
> 
> Pantelis> Fix interrupt storm on bone A4 cause by non-by-the-book
> Pantelis> interrupt handling.  While at it, added a non-NAPI mode
> Pantelis> (which is easier to debug), plus some general fixes.
> 
> 'bone A4' is also not very descriptive. There also was an A4 revision of
> the "old" beaglebone. I guess you're instead referring to a new die
> revision?
> 
> What is the impact of this change on earlier devices?
> 

Same driver works perfectly fine with the PG1.0 bone version I have.
If you take a look at the TRM it should be pretty obvious why the change
was needed.

> Pantelis> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> Pantelis> @@ -20,6 +20,7 @@ Required properties:
> Pantelis>  - cpts_clock_shift	: Denominator to convert input clock ticks into nanoseconds
> Pantelis>  - phy_id		: Specifies slave phy id
> Pantelis>  - mac-address		: Specifies slave MAC address
> Pantelis> +- disable-napi		: Disables driver NAPI
> 
> napi is not a hardware feature, so it doesn't belong here (if anything,
> it should be linux,disable-napi).
> 

Point taken.

> -- 
> Bye, Peter Korsgaard

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pantelis Antoniou Jan. 30, 2013, 8:36 a.m. UTC | #9
Hi Mugunthan,

On Jan 29, 2013, at 1:45 PM, Mugunthan V N wrote:

> On 1/28/2013 6:41 PM, Pantelis Antoniou wrote:
>> Fix interrupt storm on bone A4 cause by non-by-the-book interrupt handling.
>> While at it, added a non-NAPI mode (which is easier to debug), plus
>> some general fixes.
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> ---
>>  Documentation/devicetree/bindings/net/cpsw.txt |   1 +
>>  drivers/net/ethernet/ti/cpsw.c                 | 222 +++++++++++++++++++++----
>>  drivers/net/ethernet/ti/davinci_cpdma.c        |   4 +-
>>  drivers/net/ethernet/ti/davinci_cpdma.h        |   2 +-
>>  include/linux/platform_data/cpsw.h             |   1 +
>>  5 files changed, 194 insertions(+), 36 deletions(-)
> I have tested CPSW on AM335x EVM 1.5A with flood ping and i am not
> seeing any interrupt storm.
> Can you provide more details on how to reproduce the issue.
> 

A beaglebone prototype with the new silicon version, with the ethernet errata
fixed displays this. You can't trigger it on old silicon.

The TI people on the CC list can confirm.

> Regards
> Mugunthan V N

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mugunthan V N Jan. 30, 2013, 9:03 a.m. UTC | #10
On 1/30/2013 2:06 PM, Pantelis Antoniou wrote:
> Hi Mugunthan,
>
> On Jan 29, 2013, at 1:45 PM, Mugunthan V N wrote:
>
>> On 1/28/2013 6:41 PM, Pantelis Antoniou wrote:
>>> Fix interrupt storm on bone A4 cause by non-by-the-book interrupt handling.
>>> While at it, added a non-NAPI mode (which is easier to debug), plus
>>> some general fixes.
>>>
>>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>>> ---
>>>   Documentation/devicetree/bindings/net/cpsw.txt |   1 +
>>>   drivers/net/ethernet/ti/cpsw.c                 | 222 +++++++++++++++++++++----
>>>   drivers/net/ethernet/ti/davinci_cpdma.c        |   4 +-
>>>   drivers/net/ethernet/ti/davinci_cpdma.h        |   2 +-
>>>   include/linux/platform_data/cpsw.h             |   1 +
>>>   5 files changed, 194 insertions(+), 36 deletions(-)
>> I have tested CPSW on AM335x EVM 1.5A with flood ping and i am not
>> seeing any interrupt storm.
>> Can you provide more details on how to reproduce the issue.
>>
> A beaglebone prototype with the new silicon version, with the ethernet errata
> fixed displays this. You can't trigger it on old silicon.
>
> The TI people on the CC list can confirm.
But i have the same silicon revision (PG2.0) in my EVM and I am not 
seeing any issues. Can you
point me to the ethernet errata which you are mentioning?

Regards
Mugunthan V N
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pantelis Antoniou Jan. 30, 2013, 9:36 a.m. UTC | #11
Hi,

On Jan 30, 2013, at 11:03 AM, Mugunthan V N wrote:

> On 1/30/2013 2:06 PM, Pantelis Antoniou wrote:
>> Hi Mugunthan,
>> 
>> On Jan 29, 2013, at 1:45 PM, Mugunthan V N wrote:
>> 
>>> On 1/28/2013 6:41 PM, Pantelis Antoniou wrote:
>>>> Fix interrupt storm on bone A4 cause by non-by-the-book interrupt handling.
>>>> While at it, added a non-NAPI mode (which is easier to debug), plus
>>>> some general fixes.
>>>> 
>>>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/net/cpsw.txt |   1 +
>>>>  drivers/net/ethernet/ti/cpsw.c                 | 222 +++++++++++++++++++++----
>>>>  drivers/net/ethernet/ti/davinci_cpdma.c        |   4 +-
>>>>  drivers/net/ethernet/ti/davinci_cpdma.h        |   2 +-
>>>>  include/linux/platform_data/cpsw.h             |   1 +
>>>>  5 files changed, 194 insertions(+), 36 deletions(-)
>>> I have tested CPSW on AM335x EVM 1.5A with flood ping and i am not
>>> seeing any interrupt storm.
>>> Can you provide more details on how to reproduce the issue.
>>> 
>> A beaglebone prototype with the new silicon version, with the ethernet errata
>> fixed displays this. You can't trigger it on old silicon.
>> 
>> The TI people on the CC list can confirm.
> But i have the same silicon revision (PG2.0) in my EVM and I am not seeing any issues. Can you
> point me to the ethernet errata which you are mentioning?
> 
> Regards
> Mugunthan V N

What kernel version are you using? This is only triggered on the mainline driver.

The advisory in question: From http://www.ti.com/lit/er/sprz360c/sprz360c.pdf 

Advisory 1.0.9: "Ethernet Media Access Controller and Switch Subsystem: C0_TX_PEND 
and C0_RX_PEND Interrupts Not Connected to ARM Cortex-A8"

I bet you're using an old kernel driver with the workarounds with the timers.

If I had to guess (although I didn't use a probe or anything) is that the
interrupts are now proper level interrupts, instead of working in edge
triggered mode due to the workaround. 

Apparently the interrupt was never acked properly in the original driver 
(the sequence described in the TRM is not followed).

Looking at the TRM (spruh73g.pdf) 14.3.1.3 Interrupts in particular, the
the status registers are not read, and more damning the proper values to the 
CPDMA_EOI_VECTOR register are not written.

The original driver blindly wrote zero (cpdma_ctlr_eoi), while you have to
write different values according to the interrupt you ack.

What happened was that on the first interrupt, the interrupt was never acked,
and we had an irq storm...

Regards

-- Pantelis




--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mugunthan V N Jan. 30, 2013, 10:55 a.m. UTC | #12
On 1/30/2013 3:06 PM, Pantelis Antoniou wrote:
> Hi,
>
> On Jan 30, 2013, at 11:03 AM, Mugunthan V N wrote:
>
>> On 1/30/2013 2:06 PM, Pantelis Antoniou wrote:
>>> Hi Mugunthan,
>>>
>>> On Jan 29, 2013, at 1:45 PM, Mugunthan V N wrote:
>>>
>>>> On 1/28/2013 6:41 PM, Pantelis Antoniou wrote:
>>>>> Fix interrupt storm on bone A4 cause by non-by-the-book interrupt handling.
>>>>> While at it, added a non-NAPI mode (which is easier to debug), plus
>>>>> some general fixes.
>>>>>
>>>>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>>>>> ---
>>>>>   Documentation/devicetree/bindings/net/cpsw.txt |   1 +
>>>>>   drivers/net/ethernet/ti/cpsw.c                 | 222 +++++++++++++++++++++----
>>>>>   drivers/net/ethernet/ti/davinci_cpdma.c        |   4 +-
>>>>>   drivers/net/ethernet/ti/davinci_cpdma.h        |   2 +-
>>>>>   include/linux/platform_data/cpsw.h             |   1 +
>>>>>   5 files changed, 194 insertions(+), 36 deletions(-)
>>>> I have tested CPSW on AM335x EVM 1.5A with flood ping and i am not
>>>> seeing any interrupt storm.
>>>> Can you provide more details on how to reproduce the issue.
>>>>
>>> A beaglebone prototype with the new silicon version, with the ethernet errata
>>> fixed displays this. You can't trigger it on old silicon.
>>>
>>> The TI people on the CC list can confirm.
>> But i have the same silicon revision (PG2.0) in my EVM and I am not seeing any issues. Can you
>> point me to the ethernet errata which you are mentioning?
>>
>> Regards
>> Mugunthan V N
> What kernel version are you using? This is only triggered on the mainline driver.
>
> The advisory in question: From http://www.ti.com/lit/er/sprz360c/sprz360c.pdf
>
> Advisory 1.0.9: "Ethernet Media Access Controller and Switch Subsystem: C0_TX_PEND
> and C0_RX_PEND Interrupts Not Connected to ARM Cortex-A8"
>
> I bet you're using an old kernel driver with the workarounds with the timers.
>
> If I had to guess (although I didn't use a probe or anything) is that the
> interrupts are now proper level interrupts, instead of working in edge
> triggered mode due to the workaround.
>
> Apparently the interrupt was never acked properly in the original driver
> (the sequence described in the TRM is not followed).
>
> Looking at the TRM (spruh73g.pdf) 14.3.1.3 Interrupts in particular, the
> the status registers are not read, and more damning the proper values to the
> CPDMA_EOI_VECTOR register are not written.
>
> The original driver blindly wrote zero (cpdma_ctlr_eoi), while you have to
> write different values according to the interrupt you ack.
>
> What happened was that on the first interrupt, the interrupt was never acked,
> and we had an irq storm...
>
> Regards
>
> -- Pantelis
The above mentioned advisory is for PG1.0 and not for PG2.0
I am booting net-next kernel.

[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 3.8.0-rc5-01248-gd2ed273 
(a0131834@a0131834-linux) (gcc version 4.5.3 20110311 (prerelease) (GCC) 
) #21 SMP Wed Jan 30 163
[    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), 
cr=10c53c7d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing 
instruction cache

[root@arago /]# uname -a
Linux arago 3.8.0-rc5-01248-gd2ed273 #21 SMP Wed Jan 30 16:13:26 IST 
2013 armv7l GNU/Linux

In theory what you are mentioning is correct. I have a beagle bone black 
and yet to try it.

Regards
Mugunthan V N
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pantelis Antoniou Jan. 30, 2013, 11:08 a.m. UTC | #13
Hi Mugunthan,

On Jan 30, 2013, at 12:55 PM, Mugunthan V N wrote:

> On 1/30/2013 3:06 PM, Pantelis Antoniou wrote:
>> Hi,
>> 
>> On Jan 30, 2013, at 11:03 AM, Mugunthan V N wrote:
>> 
>>> On 1/30/2013 2:06 PM, Pantelis Antoniou wrote:
>>>> Hi Mugunthan,
>>>> 
>>>> On Jan 29, 2013, at 1:45 PM, Mugunthan V N wrote:
>>>> 
>>>>> On 1/28/2013 6:41 PM, Pantelis Antoniou wrote:
>>>>>> Fix interrupt storm on bone A4 cause by non-by-the-book interrupt handling.
>>>>>> While at it, added a non-NAPI mode (which is easier to debug), plus
>>>>>> some general fixes.
>>>>>> 
>>>>>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>>>>>> ---
>>>>>>  Documentation/devicetree/bindings/net/cpsw.txt |   1 +
>>>>>>  drivers/net/ethernet/ti/cpsw.c                 | 222 +++++++++++++++++++++----
>>>>>>  drivers/net/ethernet/ti/davinci_cpdma.c        |   4 +-
>>>>>>  drivers/net/ethernet/ti/davinci_cpdma.h        |   2 +-
>>>>>>  include/linux/platform_data/cpsw.h             |   1 +
>>>>>>  5 files changed, 194 insertions(+), 36 deletions(-)
>>>>> I have tested CPSW on AM335x EVM 1.5A with flood ping and i am not
>>>>> seeing any interrupt storm.
>>>>> Can you provide more details on how to reproduce the issue.
>>>>> 
>>>> A beaglebone prototype with the new silicon version, with the ethernet errata
>>>> fixed displays this. You can't trigger it on old silicon.
>>>> 
>>>> The TI people on the CC list can confirm.
>>> But i have the same silicon revision (PG2.0) in my EVM and I am not seeing any issues. Can you
>>> point me to the ethernet errata which you are mentioning?
>>> 
>>> Regards
>>> Mugunthan V N
>> What kernel version are you using? This is only triggered on the mainline driver.
>> 
>> The advisory in question: From http://www.ti.com/lit/er/sprz360c/sprz360c.pdf
>> 
>> Advisory 1.0.9: "Ethernet Media Access Controller and Switch Subsystem: C0_TX_PEND
>> and C0_RX_PEND Interrupts Not Connected to ARM Cortex-A8"
>> 
>> I bet you're using an old kernel driver with the workarounds with the timers.
>> 
>> If I had to guess (although I didn't use a probe or anything) is that the
>> interrupts are now proper level interrupts, instead of working in edge
>> triggered mode due to the workaround.
>> 
>> Apparently the interrupt was never acked properly in the original driver
>> (the sequence described in the TRM is not followed).
>> 
>> Looking at the TRM (spruh73g.pdf) 14.3.1.3 Interrupts in particular, the
>> the status registers are not read, and more damning the proper values to the
>> CPDMA_EOI_VECTOR register are not written.
>> 
>> The original driver blindly wrote zero (cpdma_ctlr_eoi), while you have to
>> write different values according to the interrupt you ack.
>> 
>> What happened was that on the first interrupt, the interrupt was never acked,
>> and we had an irq storm...
>> 
>> Regards
>> 
>> -- Pantelis
> The above mentioned advisory is for PG1.0 and not for PG2.0
> I am booting net-next kernel.
> 
> [    0.000000] Booting Linux on physical CPU 0x0
> [    0.000000] Linux version 3.8.0-rc5-01248-gd2ed273 (a0131834@a0131834-linux) (gcc version 4.5.3 20110311 (prerelease) (GCC) ) #21 SMP Wed Jan 30 163
> [    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
> [    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
> 
> [root@arago /]# uname -a
> Linux arago 3.8.0-rc5-01248-gd2ed273 #21 SMP Wed Jan 30 16:13:26 IST 2013 armv7l GNU/Linux
> 
> In theory what you are mentioning is correct. I have a beagle bone black and yet to try it.
> 

I don't know what kind of silicon revision you have there.

FWIW both the original bone and the black show the exact same CPU: ARMv7 lines:

bone-original:
[    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c5387d

bone-black: (with the known silicon version that has the fix)
[    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c5387d

TBH I haven't found a simple way to print out the silicon revision number.
Anyone on the list know a quick and dirty method? 

> Regards
> Mugunthan V N

Regards

-- Pantelis


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Jan. 30, 2013, 12:38 p.m. UTC | #14
On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote:
[...]
> 
> TBH I haven't found a simple way to print out the silicon revision number.
> Anyone on the list know a quick and dirty method? 
> 

You can dump the DEVICE_ID register @ 0x44e10600.
Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0.

Regards,
Vaibhav 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pantelis Antoniou Jan. 30, 2013, 12:44 p.m. UTC | #15
Hi Vaibhav,

On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote:

> On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote:
> [...]
>> 
>> TBH I haven't found a simple way to print out the silicon revision number.
>> Anyone on the list know a quick and dirty method? 
>> 
> 
> You can dump the DEVICE_ID register @ 0x44e10600.
> Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0.
> 

Thanks this works perfectly:

original-bone:
root@beaglebone:~# devmem2 0x44e10600 w
Read at address  0x44E10600 (0xb6ff4600): 0x0B94402E

bone-black:
root@beaglebone:~# devmem2 0x44e10600 w
Read at address  0x44E10600 (0xb6fcc600): 0x1B94402E

> Regards,
> Vaibhav 
> 

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Jan. 30, 2013, 1:29 p.m. UTC | #16
On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote:
> Hi Vaibhav,
> 
> On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote:
> 
> > On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote:
> > [...]
> >> 
> >> TBH I haven't found a simple way to print out the silicon revision number.
> >> Anyone on the list know a quick and dirty method? 
> >> 
> > 
> > You can dump the DEVICE_ID register @ 0x44e10600.
> > Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0.
> > 
> 
> Thanks this works perfectly:
> 
> original-bone:
> root@beaglebone:~# devmem2 0x44e10600 w
> Read at address  0x44E10600 (0xb6ff4600): 0x0B94402E
> 
> bone-black:
> root@beaglebone:~# devmem2 0x44e10600 w
> Read at address  0x44E10600 (0xb6fcc600): 0x1B94402E
> 

I just re-read the mail-chain and I am confused here.
So the patch in question is meant for Bone-A4 which has
PG1.0?

Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pantelis Antoniou Jan. 30, 2013, 1:37 p.m. UTC | #17
Hi Vaibhav,

On Jan 30, 2013, at 3:29 PM, Bedia, Vaibhav wrote:

> On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote:
>> Hi Vaibhav,
>> 
>> On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote:
>> 
>>> On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote:
>>> [...]
>>>> 
>>>> TBH I haven't found a simple way to print out the silicon revision number.
>>>> Anyone on the list know a quick and dirty method? 
>>>> 
>>> 
>>> You can dump the DEVICE_ID register @ 0x44e10600.
>>> Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0.
>>> 
>> 
>> Thanks this works perfectly:
>> 
>> original-bone:
>> root@beaglebone:~# devmem2 0x44e10600 w
>> Read at address  0x44E10600 (0xb6ff4600): 0x0B94402E
>> 
>> bone-black:
>> root@beaglebone:~# devmem2 0x44e10600 w
>> Read at address  0x44E10600 (0xb6fcc600): 0x1B94402E
>> 
> 
> I just re-read the mail-chain and I am confused here.
> So the patch in question is meant for Bone-A4 which has
> PG1.0?
> 

It is a general bug fix. The problem was discovered only on 
the bone black which has PG2.0 silicon. The driver has been
tested and it works on the original bone with PG1.0 as well.

> Regards,
> Vaibhav

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Jan. 30, 2013, 1:47 p.m. UTC | #18
Hi Antoniou,

On Wed, Jan 30, 2013 at 19:07:19, Pantelis Antoniou wrote:
> Hi Vaibhav,
> 
> On Jan 30, 2013, at 3:29 PM, Bedia, Vaibhav wrote:
> 
> > On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote:
> >> Hi Vaibhav,
> >> 
> >> On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote:
> >> 
> >>> On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote:
> >>> [...]
> >>>> 
> >>>> TBH I haven't found a simple way to print out the silicon revision number.
> >>>> Anyone on the list know a quick and dirty method? 
> >>>> 
> >>> 
> >>> You can dump the DEVICE_ID register @ 0x44e10600.
> >>> Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0.
> >>> 
> >> 
> >> Thanks this works perfectly:
> >> 
> >> original-bone:
> >> root@beaglebone:~# devmem2 0x44e10600 w
> >> Read at address  0x44E10600 (0xb6ff4600): 0x0B94402E
> >> 
> >> bone-black:
> >> root@beaglebone:~# devmem2 0x44e10600 w
> >> Read at address  0x44E10600 (0xb6fcc600): 0x1B94402E
> >> 
> > 
> > I just re-read the mail-chain and I am confused here.
> > So the patch in question is meant for Bone-A4 which has
> > PG1.0?
> > 
> 
> It is a general bug fix. The problem was discovered only on 
> the bone black which has PG2.0 silicon. The driver has been
> tested and it works on the original bone with PG1.0 as well.
> 

But Mugunthan mentioned that he doesn't see this on an EVM
with PG2.0 silicon... is there any board dependency here?

Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pantelis Antoniou Jan. 30, 2013, 1:51 p.m. UTC | #19
Hi Vaibhav,

On Jan 30, 2013, at 3:47 PM, Bedia, Vaibhav wrote:

> Hi Antoniou,
> 
> On Wed, Jan 30, 2013 at 19:07:19, Pantelis Antoniou wrote:
>> Hi Vaibhav,
>> 
>> On Jan 30, 2013, at 3:29 PM, Bedia, Vaibhav wrote:
>> 
>>> On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote:
>>>> Hi Vaibhav,
>>>> 
>>>> On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote:
>>>> 
>>>>> On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote:
>>>>> [...]
>>>>>> 
>>>>>> TBH I haven't found a simple way to print out the silicon revision number.
>>>>>> Anyone on the list know a quick and dirty method? 
>>>>>> 
>>>>> 
>>>>> You can dump the DEVICE_ID register @ 0x44e10600.
>>>>> Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0.
>>>>> 
>>>> 
>>>> Thanks this works perfectly:
>>>> 
>>>> original-bone:
>>>> root@beaglebone:~# devmem2 0x44e10600 w
>>>> Read at address  0x44E10600 (0xb6ff4600): 0x0B94402E
>>>> 
>>>> bone-black:
>>>> root@beaglebone:~# devmem2 0x44e10600 w
>>>> Read at address  0x44E10600 (0xb6fcc600): 0x1B94402E
>>>> 
>>> 
>>> I just re-read the mail-chain and I am confused here.
>>> So the patch in question is meant for Bone-A4 which has
>>> PG1.0?
>>> 
>> 
>> It is a general bug fix. The problem was discovered only on 
>> the bone black which has PG2.0 silicon. The driver has been
>> tested and it works on the original bone with PG1.0 as well.
>> 
> 
> But Mugunthan mentioned that he doesn't see this on an EVM
> with PG2.0 silicon... is there any board dependency here?
> 

I don't know, but I doubt it. How about we wait for Mugunthan to
send us what are the DEVICE_ID contents for his board.

> Regards,
> Vaibhav

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mugunthan V N Jan. 30, 2013, 1:53 p.m. UTC | #20
On 1/30/2013 7:21 PM, Pantelis Antoniou wrote:
> Hi Vaibhav,
>
> On Jan 30, 2013, at 3:47 PM, Bedia, Vaibhav wrote:
>
>> Hi Antoniou,
>>
>> On Wed, Jan 30, 2013 at 19:07:19, Pantelis Antoniou wrote:
>>> Hi Vaibhav,
>>>
>>> On Jan 30, 2013, at 3:29 PM, Bedia, Vaibhav wrote:
>>>
>>>> On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote:
>>>>> Hi Vaibhav,
>>>>>
>>>>> On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote:
>>>>>
>>>>>> On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote:
>>>>>> [...]
>>>>>>> TBH I haven't found a simple way to print out the silicon revision number.
>>>>>>> Anyone on the list know a quick and dirty method?
>>>>>>>
>>>>>> You can dump the DEVICE_ID register @ 0x44e10600.
>>>>>> Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0.
>>>>>>
>>>>> Thanks this works perfectly:
>>>>>
>>>>> original-bone:
>>>>> root@beaglebone:~# devmem2 0x44e10600 w
>>>>> Read at address  0x44E10600 (0xb6ff4600): 0x0B94402E
>>>>>
>>>>> bone-black:
>>>>> root@beaglebone:~# devmem2 0x44e10600 w
>>>>> Read at address  0x44E10600 (0xb6fcc600): 0x1B94402E
>>>>>
>>>> I just re-read the mail-chain and I am confused here.
>>>> So the patch in question is meant for Bone-A4 which has
>>>> PG1.0?
>>>>
>>> It is a general bug fix. The problem was discovered only on
>>> the bone black which has PG2.0 silicon. The driver has been
>>> tested and it works on the original bone with PG1.0 as well.
>>>
>> But Mugunthan mentioned that he doesn't see this on an EVM
>> with PG2.0 silicon... is there any board dependency here?
>>
> I don't know, but I doubt it. How about we wait for Mugunthan to
> send us what are the DEVICE_ID contents for his board.
>
>> Regards,
>> Vaibhav
> Regards
>
> -- Pantelis
>
This is the device ID which i have

[root@arago /]# devmem 0x44e10600
0x1B94402E

Regards
Mugunthan V N
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pantelis Antoniou Jan. 30, 2013, 1:55 p.m. UTC | #21
Hi Mugunthan,

On Jan 30, 2013, at 3:53 PM, Mugunthan V N wrote:

> On 1/30/2013 7:21 PM, Pantelis Antoniou wrote:
>> Hi Vaibhav,
>> 
>> On Jan 30, 2013, at 3:47 PM, Bedia, Vaibhav wrote:
>> 
>>> Hi Antoniou,
>>> 
>>> On Wed, Jan 30, 2013 at 19:07:19, Pantelis Antoniou wrote:
>>>> Hi Vaibhav,
>>>> 
>>>> On Jan 30, 2013, at 3:29 PM, Bedia, Vaibhav wrote:
>>>> 
>>>>> On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote:
>>>>>> Hi Vaibhav,
>>>>>> 
>>>>>> On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote:
>>>>>> 
>>>>>>> On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote:
>>>>>>> [...]
>>>>>>>> TBH I haven't found a simple way to print out the silicon revision number.
>>>>>>>> Anyone on the list know a quick and dirty method?
>>>>>>>> 
>>>>>>> You can dump the DEVICE_ID register @ 0x44e10600.
>>>>>>> Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0.
>>>>>>> 
>>>>>> Thanks this works perfectly:
>>>>>> 
>>>>>> original-bone:
>>>>>> root@beaglebone:~# devmem2 0x44e10600 w
>>>>>> Read at address  0x44E10600 (0xb6ff4600): 0x0B94402E
>>>>>> 
>>>>>> bone-black:
>>>>>> root@beaglebone:~# devmem2 0x44e10600 w
>>>>>> Read at address  0x44E10600 (0xb6fcc600): 0x1B94402E
>>>>>> 
>>>>> I just re-read the mail-chain and I am confused here.
>>>>> So the patch in question is meant for Bone-A4 which has
>>>>> PG1.0?
>>>>> 
>>>> It is a general bug fix. The problem was discovered only on
>>>> the bone black which has PG2.0 silicon. The driver has been
>>>> tested and it works on the original bone with PG1.0 as well.
>>>> 
>>> But Mugunthan mentioned that he doesn't see this on an EVM
>>> with PG2.0 silicon... is there any board dependency here?
>>> 
>> I don't know, but I doubt it. How about we wait for Mugunthan to
>> send us what are the DEVICE_ID contents for his board.
>> 
>>> Regards,
>>> Vaibhav
>> Regards
>> 
>> -- Pantelis
>> 
> This is the device ID which i have
> 
> [root@arago /]# devmem 0x44e10600
> 0x1B94402E
> 

No clue what's the difference of the black with the EVM, and why this happens.
The fix is valid anyway.

> Regards
> Mugunthan V N

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mugunthan V N Jan. 30, 2013, 2:03 p.m. UTC | #22
On 1/30/2013 7:25 PM, Pantelis Antoniou wrote:
> Hi Mugunthan,
>
> On Jan 30, 2013, at 3:53 PM, Mugunthan V N wrote:
>
>> On 1/30/2013 7:21 PM, Pantelis Antoniou wrote:
>>> Hi Vaibhav,
>>>
>>> On Jan 30, 2013, at 3:47 PM, Bedia, Vaibhav wrote:
>>>
>>>> Hi Antoniou,
>>>>
>>>> On Wed, Jan 30, 2013 at 19:07:19, Pantelis Antoniou wrote:
>>>>> Hi Vaibhav,
>>>>>
>>>>> On Jan 30, 2013, at 3:29 PM, Bedia, Vaibhav wrote:
>>>>>
>>>>>> On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote:
>>>>>>> Hi Vaibhav,
>>>>>>>
>>>>>>> On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote:
>>>>>>>
>>>>>>>> On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote:
>>>>>>>> [...]
>>>>>>>>> TBH I haven't found a simple way to print out the silicon revision number.
>>>>>>>>> Anyone on the list know a quick and dirty method?
>>>>>>>>>
>>>>>>>> You can dump the DEVICE_ID register @ 0x44e10600.
>>>>>>>> Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0.
>>>>>>>>
>>>>>>> Thanks this works perfectly:
>>>>>>>
>>>>>>> original-bone:
>>>>>>> root@beaglebone:~# devmem2 0x44e10600 w
>>>>>>> Read at address  0x44E10600 (0xb6ff4600): 0x0B94402E
>>>>>>>
>>>>>>> bone-black:
>>>>>>> root@beaglebone:~# devmem2 0x44e10600 w
>>>>>>> Read at address  0x44E10600 (0xb6fcc600): 0x1B94402E
>>>>>>>
>>>>>> I just re-read the mail-chain and I am confused here.
>>>>>> So the patch in question is meant for Bone-A4 which has
>>>>>> PG1.0?
>>>>>>
>>>>> It is a general bug fix. The problem was discovered only on
>>>>> the bone black which has PG2.0 silicon. The driver has been
>>>>> tested and it works on the original bone with PG1.0 as well.
>>>>>
>>>> But Mugunthan mentioned that he doesn't see this on an EVM
>>>> with PG2.0 silicon... is there any board dependency here?
>>>>
>>> I don't know, but I doubt it. How about we wait for Mugunthan to
>>> send us what are the DEVICE_ID contents for his board.
>>>
>>>> Regards,
>>>> Vaibhav
>>> Regards
>>>
>>> -- Pantelis
>>>
>> This is the device ID which i have
>>
>> [root@arago /]# devmem 0x44e10600
>> 0x1B94402E
>>
> No clue what's the difference of the black with the EVM, and why this happens.
> The fix is valid anyway.
>
>> Regards
>> Mugunthan V N
> Regards
>
> -- Pantelis
>
I will try to run overnight ethernet test in EVM and will update by 
tomorrow.

Regards
Mugunthan V N
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mugunthan V N Jan. 30, 2013, 7:27 p.m. UTC | #23
Hi Antoniou

On 1/30/2013 7:25 PM, Pantelis Antoniou wrote:
> Hi Mugunthan,
>
> On Jan 30, 2013, at 3:53 PM, Mugunthan V N wrote:
>
>> On 1/30/2013 7:21 PM, Pantelis Antoniou wrote:
>>> Hi Vaibhav,
>>>
>>> On Jan 30, 2013, at 3:47 PM, Bedia, Vaibhav wrote:
>>>
>>>> Hi Antoniou,
>>>>
>>>> On Wed, Jan 30, 2013 at 19:07:19, Pantelis Antoniou wrote:
>>>>> Hi Vaibhav,
>>>>>
>>>>> On Jan 30, 2013, at 3:29 PM, Bedia, Vaibhav wrote:
>>>>>
>>>>>> On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote:
>>>>>>> Hi Vaibhav,
>>>>>>>
>>>>>>> On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote:
>>>>>>>
>>>>>>>> On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote:
>>>>>>>> [...]
>>>>>>>>> TBH I haven't found a simple way to print out the silicon revision number.
>>>>>>>>> Anyone on the list know a quick and dirty method?
>>>>>>>>>
>>>>>>>> You can dump the DEVICE_ID register @ 0x44e10600.
>>>>>>>> Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0.
>>>>>>>>
>>>>>>> Thanks this works perfectly:
>>>>>>>
>>>>>>> original-bone:
>>>>>>> root@beaglebone:~# devmem2 0x44e10600 w
>>>>>>> Read at address  0x44E10600 (0xb6ff4600): 0x0B94402E
>>>>>>>
>>>>>>> bone-black:
>>>>>>> root@beaglebone:~# devmem2 0x44e10600 w
>>>>>>> Read at address  0x44E10600 (0xb6fcc600): 0x1B94402E
>>>>>>>
>>>>>> I just re-read the mail-chain and I am confused here.
>>>>>> So the patch in question is meant for Bone-A4 which has
>>>>>> PG1.0?
>>>>>>
>>>>> It is a general bug fix. The problem was discovered only on
>>>>> the bone black which has PG2.0 silicon. The driver has been
>>>>> tested and it works on the original bone with PG1.0 as well.
>>>>>
>>>> But Mugunthan mentioned that he doesn't see this on an EVM
>>>> with PG2.0 silicon... is there any board dependency here?
>>>>
>>> I don't know, but I doubt it. How about we wait for Mugunthan to
>>> send us what are the DEVICE_ID contents for his board.
>>>
>>>> Regards,
>>>> Vaibhav
>>> Regards
>>>
>>> -- Pantelis
>>>
>> This is the device ID which i have
>>
>> [root@arago /]# devmem 0x44e10600
>> 0x1B94402E
>>
> No clue what's the difference of the black with the EVM, and why this happens.
> The fix is valid anyway.
I am able to see the issue in EVM also by comparing the number of rx  
interrupt received
from CPSW and no of packets actually received. But this can be resolved 
by acknowledging
the interrupt in cpdma driver.

Regards
Mugunthan V N
>> Regards
>> Mugunthan V N
> Regards
>
> -- Pantelis
>

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 6ddd028..d46b293 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -20,6 +20,7 @@  Required properties:
 - cpts_clock_shift	: Denominator to convert input clock ticks into nanoseconds
 - phy_id		: Specifies slave phy id
 - mac-address		: Specifies slave MAC address
+- disable-napi		: Disables driver NAPI
 
 Optional properties:
 - ti,hwmods		: Must be "cpgmac0"
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 40aff68..b6ca4af 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -148,10 +148,37 @@  struct cpsw_wr_regs {
 	u32	soft_reset;
 	u32	control;
 	u32	int_control;
-	u32	rx_thresh_en;
-	u32	rx_en;
-	u32	tx_en;
-	u32	misc_en;
+	u32	c0_rx_thresh_en;
+	u32	c0_rx_en;
+	u32	c0_tx_en;
+	u32	c0_misc_en;
+	u32	c1_rx_thresh_en;
+	u32	c1_rx_en;
+	u32	c1_tx_en;
+	u32	c1_misc_en;
+	u32	c2_rx_thresh_en;
+	u32	c2_rx_en;
+	u32	c2_tx_en;
+	u32	c2_misc_en;
+	u32	c0_rx_thresh_stat;
+	u32	c0_rx_stat;
+	u32	c0_tx_stat;
+	u32	c0_misc_stat;
+	u32	c1_rx_thresh_stat;
+	u32	c1_rx_stat;
+	u32	c1_tx_stat;
+	u32	c1_misc_stat;
+	u32	c2_rx_thresh_stat;
+	u32	c2_rx_stat;
+	u32	c2_tx_stat;
+	u32	c2_misc_stat;
+	u32	c0_rx_imax;
+	u32	c0_tx_imax;
+	u32	c1_rx_imax;
+	u32	c1_tx_imax;
+	u32	c2_rx_imax;
+	u32	c2_tx_imax;
+	u32	rgmii_ctl;
 };
 
 struct cpsw_ss_regs {
@@ -352,8 +379,8 @@  static void cpsw_ndo_set_rx_mode(struct net_device *ndev)
 
 static void cpsw_intr_enable(struct cpsw_priv *priv)
 {
-	__raw_writel(0xFF, &priv->wr_regs->tx_en);
-	__raw_writel(0xFF, &priv->wr_regs->rx_en);
+	__raw_writel(0xFF, &priv->wr_regs->c0_tx_en);
+	__raw_writel(0xFF, &priv->wr_regs->c0_rx_en);
 
 	cpdma_ctlr_int_ctrl(priv->dma, true);
 	return;
@@ -361,8 +388,8 @@  static void cpsw_intr_enable(struct cpsw_priv *priv)
 
 static void cpsw_intr_disable(struct cpsw_priv *priv)
 {
-	__raw_writel(0, &priv->wr_regs->tx_en);
-	__raw_writel(0, &priv->wr_regs->rx_en);
+	__raw_writel(0, &priv->wr_regs->c0_tx_en);
+	__raw_writel(0, &priv->wr_regs->c0_rx_en);
 
 	cpdma_ctlr_int_ctrl(priv->dma, false);
 	return;
@@ -399,7 +426,10 @@  void cpsw_rx_handler(void *token, int len, int status)
 		skb_put(skb, len);
 		cpts_rx_timestamp(&priv->cpts, skb);
 		skb->protocol = eth_type_trans(skb, ndev);
-		netif_receive_skb(skb);
+		if (priv->data.disable_napi)
+			netif_rx(skb);
+		else
+			netif_receive_skb(skb);
 		priv->stats.rx_bytes += len;
 		priv->stats.rx_packets++;
 		skb = NULL;
@@ -431,6 +461,7 @@  static irqreturn_t cpsw_interrupt(int irq, void *dev_id)
 		cpsw_disable_irq(priv);
 		napi_schedule(&priv->napi);
 	}
+
 	return IRQ_HANDLED;
 }
 
@@ -445,23 +476,104 @@  static inline int cpsw_get_slave_port(struct cpsw_priv *priv, u32 slave_num)
 static int cpsw_poll(struct napi_struct *napi, int budget)
 {
 	struct cpsw_priv	*priv = napi_to_priv(napi);
-	int			num_tx, num_rx;
+	int			num_tx, num_rx, num_total_tx, num_total_rx;
+	int			budget_left;
+
+	budget_left = budget;
 
-	num_tx = cpdma_chan_process(priv->txch, 128);
-	num_rx = cpdma_chan_process(priv->rxch, budget);
+	/* read status and throw away */
+	(void)__raw_readl(&priv->wr_regs->c0_tx_stat);
+
+	/* handle all transmits */
+	num_total_tx = 0;
+	while (budget_left > 0 &&
+		(num_tx = cpdma_chan_process(priv->txch, 128)) > 0) {
+		budget_left -= num_tx;
+		num_total_tx += num_tx;
+	}
 
-	if (num_rx || num_tx)
-		cpsw_dbg(priv, intr, "poll %d rx, %d tx pkts\n",
-			 num_rx, num_tx);
+	if (num_total_tx > 0 && budget_left > 0)
+		cpdma_ctlr_eoi(priv->dma, 0x02);
+
+	/* read status and throw away */
+	(void)__raw_readl(&priv->wr_regs->c0_rx_stat);
+
+	/* handle all receives */
+	num_total_rx = 0;
+	while (budget_left > 0 &&
+		(num_rx = cpdma_chan_process(priv->rxch, budget_left)) > 0) {
+		budget_left -= num_rx;
+		num_total_rx += num_rx;
+	}
 
-	if (num_rx < budget) {
+	if (num_total_rx > 0 && budget_left > 0)
+		cpdma_ctlr_eoi(priv->dma, 0x01);
+
+	if ((num_total_rx + num_total_tx) < budget) {
 		napi_complete(napi);
 		cpsw_intr_enable(priv);
-		cpdma_ctlr_eoi(priv->dma);
 		cpsw_enable_irq(priv);
 	}
 
-	return num_rx;
+	return num_total_rx + num_total_rx;
+}
+
+static irqreturn_t cpsw_rx_thresh_pend_irq(int irq, void *dev_id)
+{
+	struct cpsw_priv *priv = dev_id;
+
+	(void)priv;
+
+	/* not handling this interrupt yet */
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t cpsw_rx_pend_irq(int irq, void *dev_id)
+{
+	struct cpsw_priv *priv = dev_id;
+	int num_rx, total_rx;
+	u32 rx_stat;
+
+	rx_stat = __raw_readl(&priv->wr_regs->c0_rx_stat) & 0xff;
+	if (rx_stat == 0)
+		return IRQ_NONE;
+
+	total_rx = 0;
+	while ((num_rx = cpdma_chan_process(priv->rxch,
+					priv->data.rx_descs)) > 0)
+		total_rx += num_rx;
+
+	cpdma_ctlr_eoi(priv->dma, 0x01);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t cpsw_tx_pend_irq(int irq, void *dev_id)
+{
+	struct cpsw_priv *priv = dev_id;
+	int num_tx, total_tx;
+	u32 tx_stat;
+
+	tx_stat = __raw_readl(&priv->wr_regs->c0_tx_stat) & 0xff;
+	if (tx_stat == 0)
+		return IRQ_NONE;
+
+	total_tx = 0;
+	while ((num_tx = cpdma_chan_process(priv->txch, 128)) > 0)
+		total_tx += num_tx;
+
+	cpdma_ctlr_eoi(priv->dma, 0x02);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t cpsw_misc_pend_irq(int irq, void *dev_id)
+{
+	struct cpsw_priv *priv = dev_id;
+
+	(void)priv;
+	/* not handling this interrupt yet */
+	return IRQ_HANDLED;
 }
 
 static inline void soft_reset(const char *module, void __iomem *reg)
@@ -678,8 +790,10 @@  static int cpsw_ndo_open(struct net_device *ndev)
 
 	cpdma_ctlr_start(priv->dma);
 	cpsw_intr_enable(priv);
-	napi_enable(&priv->napi);
-	cpdma_ctlr_eoi(priv->dma);
+	if (!priv->data.disable_napi)
+		napi_enable(&priv->napi);
+	cpdma_ctlr_eoi(priv->dma, 0x01);
+	cpdma_ctlr_eoi(priv->dma, 0x02);
 
 	return 0;
 }
@@ -698,8 +812,10 @@  static int cpsw_ndo_stop(struct net_device *ndev)
 	struct cpsw_priv *priv = netdev_priv(ndev);
 
 	cpsw_info(priv, ifdown, "shutting down cpsw device\n");
+	cpsw_disable_irq(priv);
 	netif_stop_queue(priv->ndev);
-	napi_disable(&priv->napi);
+	if (!priv->data.disable_napi)
+		napi_disable(&priv->napi);
 	netif_carrier_off(priv->ndev);
 	cpsw_intr_disable(priv);
 	cpdma_ctlr_int_ctrl(priv->dma, false);
@@ -901,7 +1017,8 @@  static void cpsw_ndo_tx_timeout(struct net_device *ndev)
 	cpdma_chan_start(priv->txch);
 	cpdma_ctlr_int_ctrl(priv->dma, true);
 	cpsw_intr_enable(priv);
-	cpdma_ctlr_eoi(priv->dma);
+	cpdma_ctlr_eoi(priv->dma, 0x01);
+	cpdma_ctlr_eoi(priv->dma, 0x02);
 }
 
 static struct net_device_stats *cpsw_ndo_get_stats(struct net_device *ndev)
@@ -917,14 +1034,21 @@  static void cpsw_ndo_poll_controller(struct net_device *ndev)
 
 	cpsw_intr_disable(priv);
 	cpdma_ctlr_int_ctrl(priv->dma, false);
-	cpsw_interrupt(ndev->irq, priv);
+	if (!priv->data.disable_napi)
+		cpsw_interrupt(ndev->irq, priv);
+	else {
+		/* bah! */
+		cpsw_rx_pend_irq(ndev->irq, priv);
+		cpsw_tx_pend_irq(ndev->irq, priv);
+	}
 	cpdma_ctlr_int_ctrl(priv->dma, true);
 	cpsw_intr_enable(priv);
-	cpdma_ctlr_eoi(priv->dma);
+	cpdma_ctlr_eoi(priv->dma, 0x01);
+	cpdma_ctlr_eoi(priv->dma, 0x02);
 }
 #endif
 
-static const struct net_device_ops cpsw_netdev_ops = {
+static struct net_device_ops cpsw_netdev_ops = {
 	.ndo_open		= cpsw_ndo_open,
 	.ndo_stop		= cpsw_ndo_stop,
 	.ndo_start_xmit		= cpsw_ndo_start_xmit,
@@ -1079,6 +1203,9 @@  static int cpsw_probe_dt(struct cpsw_platform_data *data,
 	}
 	data->bd_ram_size = prop;
 
+	if (of_property_read_bool(node, "disable-napi"))
+		data->disable_napi = 1;
+
 	if (of_property_read_u32(node, "rx_descs", &prop)) {
 		pr_err("Missing rx_descs property in the DT.\n");
 		ret = -EINVAL;
@@ -1136,6 +1263,22 @@  error_ret:
 	return ret;
 }
 
+static irq_handler_t cpsw_get_irq_handler(struct cpsw_priv *priv, int irq_idx)
+{
+	static const irq_handler_t non_napi_irq_tab[4] = {
+		cpsw_rx_thresh_pend_irq, cpsw_rx_pend_irq,
+		cpsw_tx_pend_irq, cpsw_misc_pend_irq
+	};
+
+	if ((unsigned int)irq_idx >= 4)
+		return NULL;
+
+	if (!priv->data.disable_napi)
+		return cpsw_interrupt;
+
+	return non_napi_irq_tab[irq_idx];
+}
+
 static int cpsw_probe(struct platform_device *pdev)
 {
 	struct cpsw_platform_data	*data = pdev->dev.platform_data;
@@ -1146,7 +1289,8 @@  static int cpsw_probe(struct platform_device *pdev)
 	void __iomem			*ss_regs, *wr_regs;
 	struct resource			*res;
 	u32 slave_offset, sliver_offset, slave_size;
-	int ret = 0, i, k = 0;
+	irq_handler_t			irqh;
+	int ret = 0, i, j, k = 0;
 
 	ndev = alloc_etherdev(sizeof(struct cpsw_priv));
 	if (!ndev) {
@@ -1333,24 +1477,36 @@  static int cpsw_probe(struct platform_device *pdev)
 		goto clean_ale_ret;
 	}
 
-	while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k))) {
-		for (i = res->start; i <= res->end; i++) {
-			if (request_irq(i, cpsw_interrupt, IRQF_DISABLED,
+	dev_info(&pdev->dev, "NAPI %s\n", priv->data.disable_napi ?
+			"disabled" : "enabled");
+
+	/* get interrupts */
+	j = k = 0;
+	while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, j++))) {
+		for (i = res->start; k < 4 && i <= res->end; i++) {
+			irqh = cpsw_get_irq_handler(priv, k);
+			if (irqh == NULL) {
+				dev_err(&pdev->dev, "Unable to get handler "
+						"for #%d (%d)\n", k, i);
+				goto clean_ale_ret;
+			}
+			if (request_irq(i, irqh, IRQF_DISABLED,
 					dev_name(&pdev->dev), priv)) {
 				dev_err(priv->dev, "error attaching irq\n");
 				goto clean_ale_ret;
 			}
-			priv->irqs_table[k] = i;
-			priv->num_irqs = k;
+			priv->irqs_table[k++] = i;
 		}
-		k++;
 	}
+	priv->num_irqs = k;
 
 	ndev->flags |= IFF_ALLMULTI;	/* see cpsw_ndo_change_rx_flags() */
 
 	ndev->netdev_ops = &cpsw_netdev_ops;
 	SET_ETHTOOL_OPS(ndev, &cpsw_ethtool_ops);
-	netif_napi_add(ndev, &priv->napi, cpsw_poll, CPSW_POLL_WEIGHT);
+
+	if (!priv->data.disable_napi)
+		netif_napi_add(ndev, &priv->napi, cpsw_poll, CPSW_POLL_WEIGHT);
 
 	/* register the network device */
 	SET_NETDEV_DEV(ndev, &pdev->dev);
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index 4995673..6effab2 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -474,9 +474,9 @@  int cpdma_ctlr_int_ctrl(struct cpdma_ctlr *ctlr, bool enable)
 	return 0;
 }
 
-void cpdma_ctlr_eoi(struct cpdma_ctlr *ctlr)
+void cpdma_ctlr_eoi(struct cpdma_ctlr *ctlr, u32 value)
 {
-	dma_reg_write(ctlr, CPDMA_MACEOIVECTOR, 0);
+	dma_reg_write(ctlr, CPDMA_MACEOIVECTOR, value);
 }
 
 struct cpdma_chan *cpdma_chan_create(struct cpdma_ctlr *ctlr, int chan_num,
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.h b/drivers/net/ethernet/ti/davinci_cpdma.h
index afa19a0..b7c097d 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.h
+++ b/drivers/net/ethernet/ti/davinci_cpdma.h
@@ -86,7 +86,7 @@  int cpdma_chan_submit(struct cpdma_chan *chan, void *token, void *data,
 int cpdma_chan_process(struct cpdma_chan *chan, int quota);
 
 int cpdma_ctlr_int_ctrl(struct cpdma_ctlr *ctlr, bool enable);
-void cpdma_ctlr_eoi(struct cpdma_ctlr *ctlr);
+void cpdma_ctlr_eoi(struct cpdma_ctlr *ctlr, u32 value);
 int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable);
 
 enum cpdma_control {
diff --git a/include/linux/platform_data/cpsw.h b/include/linux/platform_data/cpsw.h
index 24368a2..be5b27e 100644
--- a/include/linux/platform_data/cpsw.h
+++ b/include/linux/platform_data/cpsw.h
@@ -35,6 +35,7 @@  struct cpsw_platform_data {
 	u32	bd_ram_size;  /*buffer descriptor ram size */
 	u32	rx_descs;	/* Number of Rx Descriptios */
 	u32	mac_control;	/* Mac control register */
+	int	disable_napi;	/* disable NAPI */
 };
 
 #endif /* __CPSW_H__ */