[PATCH 0/5] Blackfin MAC updates for 2.6.31

[PATCH 0/5] Blackfin MAC updates for 2.6.31

[Mike Frysinger]

Bug fixes and MII lovin galore.

Graf Yang (2):
  netdev: bfin_mac: add Blackfin MII bus to platform bus to allow DSA
    access
  netdev: bfin_mac: fix crash when unloading module

Michael Hennerich (2):
  netdev: bfin_mac: Blackfin EMAC interrupt may not be shared
  netdev: bfin_mac: fix performance issue found by netperf

Sonic Zhang (1):
  netdev: bfin_mac: fix malformed UDP packet transmission when polling
    with KGDB

 drivers/net/bfin_mac.c |  232 +++++++++++++++++++++++------------------------
 1 files changed, 114 insertions(+), 118 deletions(-)

[PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[Mike Frysinger]

From: Michael Hennerich <michael.hennerich@analog.com>

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
 drivers/net/bfin_mac.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 9f971ed..1905532 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -1108,7 +1108,7 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
 	/* now, enable interrupts */
 	/* register irq handler */
 	rc = request_irq(IRQ_MAC_RX, bfin_mac_interrupt,
-			IRQF_DISABLED | IRQF_SHARED, "EMAC_RX", ndev);
+			IRQF_DISABLED, "EMAC_RX", ndev);
 	if (rc) {
 		dev_err(&pdev->dev, "Cannot request Blackfin MAC RX IRQ!\n");
 		rc = -EBUSY;
-- 
1.6.3.1

Re: [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[David Miller]

From: Mike Frysinger <vapier@gentoo.org>
Date: Wed, 27 May 2009 02:10:11 -0400

> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>

I'm not applying this.

Either take the time to make this interrupt handler cope with shared
interrupts (the most preferred) or take the time to write an
excruciatingly long winded and explicit commit log message explaining
why that's not possible.

And I want the commit log message path to be as painful as possible so
that you choose to fix this driver's IRQ handler to behave sanely
instead. :-)

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[Mike Frysinger]

On Fri, May 29, 2009 at 05:01, David Miller wrote:
> From: Mike Frysinger <vapier@gentoo.org>
>> From: Michael Hennerich <michael.hennerich@analog.com>
>>
>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> Signed-off-by: Bryan Wu <cooloney@kernel.org>
>
> I'm not applying this.
>
> Either take the time to make this interrupt handler cope with shared
> interrupts (the most preferred) or take the time to write an
> excruciatingly long winded and explicit commit log message explaining
> why that's not possible.
>
> And I want the commit log message path to be as painful as possible so
> that you choose to fix this driver's IRQ handler to behave sanely
> instead. :-)

the hardware has a dedicated internal IRQ line for the Blackfin MAC.
it makes no sense to mark it as shared.  there is nothing it could
possibly be shared with.
-mike

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[David Miller]

From: Mike Frysinger <vapier.adi@gmail.com>
Date: Fri, 29 May 2009 06:49:42 -0400

> the hardware has a dedicated internal IRQ line for the Blackfin MAC.
> it makes no sense to mark it as shared.  there is nothing it could
> possibly be shared with.

Then why bother messing with the flag away at all?  It doesn't fix
anything.

RE: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[Hennerich, Michael]

>From: David Miller [mailto:davem@davemloft.net]
>From: Mike Frysinger <vapier.adi@gmail.com>
>Date: Fri, 29 May 2009 06:49:42 -0400
>
>> the hardware has a dedicated internal IRQ line for the Blackfin MAC.
>> it makes no sense to mark it as shared.  there is nothing it could
>> possibly be shared with.
>
>Then why bother messing with the flag away at all?  It doesn't fix
>anything.

Well - it fixes a bug seen with CONFIG_DEBUG_SHIRQ.
CONFIG_DEBUG_SHIRQ generates spurious fake interrupts and 
checks if the handler can cope with them. 
This is basically a good thing - however only for real shared
interrupts.

Our handler isn't prepared to handle such fake interrupts.
So two options:
1) Set the proper IRQF flags.
2) Fix the ISR to handle spurious interrupts.

This patch chose option 1) 

-Michael 
 

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[David Miller]

From: "Hennerich, Michael" <Michael.Hennerich@analog.com>
Date: Fri, 29 May 2009 12:12:49 +0100

> Our handler isn't prepared to handle such fake interrupts.
> So two options:
> 1) Set the proper IRQF flags.
> 2) Fix the ISR to handle spurious interrupts.
> 
> This patch chose option 1) 

And to go back to my initial reply, I'm saying to do the
"right thing" and go with #2 if you can :-)

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[Mike Frysinger]

On Fri, May 29, 2009 at 07:14, David Miller wrote:
> From: "Hennerich, Michael" <Michael.Hennerich@analog.com>
>> Our handler isn't prepared to handle such fake interrupts.
>> So two options:
>> 1) Set the proper IRQF flags.
>> 2) Fix the ISR to handle spurious interrupts.
>>
>> This patch chose option 1)
>
> And to go back to my initial reply, I'm saying to do the
> "right thing" and go with #2 if you can :-)

and the answer is that doesnt make sense.  why implement extra over
head in an interrupt handler to support an operating mode the hardware
can never support.  if the IRQ could actually be shared, then sure, we
should check things.  but it cant, so you're suggesting we waste time
in an IRQ handler for absolutely no gain.
-mike

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[David Miller]

From: Mike Frysinger <vapier.adi@gmail.com>
Date: Fri, 29 May 2009 07:22:05 -0400

> why implement extra over head in an interrupt handler to support an
> operating mode the hardware can never support.

What overhead?  It should be pretty easy to see if the device
is really indicating an interrupt or not :-)

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[Mike Frysinger]

On Fri, May 29, 2009 at 07:28, David Miller wrote:
> From: Mike Frysinger <vapier.adi@gmail.com>
>> why implement extra over head in an interrupt handler to support an
>> operating mode the hardware can never support.
>
> What overhead?  It should be pretty easy to see if the device
> is really indicating an interrupt or not :-)

which would involve reading system mmrs which are clocked at the
system frequency and thus make the core stall
-mike

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[David Miller]

From: Mike Frysinger <vapier.adi@gmail.com>
Date: Fri, 29 May 2009 07:45:05 -0400

> On Fri, May 29, 2009 at 07:28, David Miller wrote:
>> From: Mike Frysinger <vapier.adi@gmail.com>
>>> why implement extra over head in an interrupt handler to support an
>>> operating mode the hardware can never support.
>>
>> What overhead?  It should be pretty easy to see if the device
>> is really indicating an interrupt or not :-)
> 
> which would involve reading system mmrs which are clocked at the
> system frequency and thus make the core stall

And the core doesn't stall reading in these cache lines that the chip
has just DMA'd to?

I think you're throwing the baby out with the bath water :-)

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[Mike Frysinger]

On Fri, May 29, 2009 at 18:09, David Miller wrote:
> From: Mike Frysinger <vapier.adi@gmail.com>
>> On Fri, May 29, 2009 at 07:28, David Miller wrote:
>>> From: Mike Frysinger <vapier.adi@gmail.com>
>>>> why implement extra over head in an interrupt handler to support an
>>>> operating mode the hardware can never support.
>>>
>>> What overhead?  It should be pretty easy to see if the device
>>> is really indicating an interrupt or not :-)
>>
>> which would involve reading system mmrs which are clocked at the
>> system frequency and thus make the core stall
>
> And the core doesn't stall reading in these cache lines that the chip
> has just DMA'd to?

the difference is that one of these is required in order for anything
to get done and the other is always useless noise.  also, the Blackfin
core does do speculative data fetching on external memory, but not
MMRs, so the stalling due to the data cache line fills will be
mitigated unlike the useless MMR reads.
-mike

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[David Miller]

From: Mike Frysinger <vapier.adi@gmail.com>
Date: Fri, 29 May 2009 18:13:02 -0400

> the difference is that one of these is required in order for anything
> to get done and the other is always useless noise.  also, the Blackfin
> core does do speculative data fetching on external memory, but not
> MMRs, so the stalling due to the data cache line fills will be
> mitigated unlike the useless MMR reads.

You only need to read the register if the descriptor status is not
ready yet, and you're just spinning in that case anyways.

I doubt it makes any real difference if implemented properly.

But I guess it's more fun to speculate than to actually try it out.
:-/

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[Mike Frysinger]

On Fri, May 29, 2009 at 18:18, David Miller wrote:
> From: Mike Frysinger <vapier.adi@gmail.com>
>> the difference is that one of these is required in order for anything
>> to get done and the other is always useless noise.  also, the Blackfin
>> core does do speculative data fetching on external memory, but not
>> MMRs, so the stalling due to the data cache line fills will be
>> mitigated unlike the useless MMR reads.
>
> You only need to read the register if the descriptor status is not
> ready yet, and you're just spinning in that case anyways.
>
> I doubt it makes any real difference if implemented properly.
>
> But I guess it's more fun to speculate than to actually try it out.
> :-/

and i guess it's more fun to waste time attempting to support an
operating mode that the hardware has never and most likely will never
support
-mike

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[David Miller]

From: Mike Frysinger <vapier.adi@gmail.com>
Date: Fri, 29 May 2009 18:21:41 -0400

> On Fri, May 29, 2009 at 18:18, David Miller wrote:
>> From: Mike Frysinger <vapier.adi@gmail.com>
>>> the difference is that one of these is required in order for anything
>>> to get done and the other is always useless noise.  also, the Blackfin
>>> core does do speculative data fetching on external memory, but not
>>> MMRs, so the stalling due to the data cache line fills will be
>>> mitigated unlike the useless MMR reads.
>>
>> You only need to read the register if the descriptor status is not
>> ready yet, and you're just spinning in that case anyways.
>>
>> I doubt it makes any real difference if implemented properly.
>>
>> But I guess it's more fun to speculate than to actually try it out.
>> :-/
> 
> and i guess it's more fun to waste time attempting to support an
> operating mode that the hardware has never and most likely will never
> support

I guess I'll apply your patch :-)

Re: [Uclinux-dist-devel] [PATCH 1/5] netdev: bfin_mac: Blackfin EMAC interrupt may not be shared

[Mike Frysinger]

On Fri, May 29, 2009 at 18:45, David Miller wrote:
> From: Mike Frysinger <vapier.adi@gmail.com>
>> On Fri, May 29, 2009 at 18:18, David Miller wrote:
>>> From: Mike Frysinger <vapier.adi@gmail.com>
>>>> the difference is that one of these is required in order for anything
>>>> to get done and the other is always useless noise.  also, the Blackfin
>>>> core does do speculative data fetching on external memory, but not
>>>> MMRs, so the stalling due to the data cache line fills will be
>>>> mitigated unlike the useless MMR reads.
>>>
>>> You only need to read the register if the descriptor status is not
>>> ready yet, and you're just spinning in that case anyways.
>>>
>>> I doubt it makes any real difference if implemented properly.
>>>
>>> But I guess it's more fun to speculate than to actually try it out.
>>> :-/
>>
>> and i guess it's more fun to waste time attempting to support an
>> operating mode that the hardware has never and most likely will never
>> support
>
> I guess I'll apply your patch :-)

thanks ... i thought you might force me to implement it anyways even
though i really really dont want it ;)
-mike

[PATCH 1/4 v2] netdev: bfin_mac: drop useless IRQF_SHARED from Blackfin EMAC interrupt

[Mike Frysinger]

From: Michael Hennerich <michael.hennerich@analog.com>

The IRQ used by the Blackfin EMAC is internal to the peripheral and cannot
be used to generate any other interrupt, so there is no point in marking it
as IRQF_SHARED.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
v2
	- improve changelog

 drivers/net/bfin_mac.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 9f971ed..1905532 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -1108,7 +1108,7 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
 	/* now, enable interrupts */
 	/* register irq handler */
 	rc = request_irq(IRQ_MAC_RX, bfin_mac_interrupt,
-			IRQF_DISABLED | IRQF_SHARED, "EMAC_RX", ndev);
+			IRQF_DISABLED, "EMAC_RX", ndev);
 	if (rc) {
 		dev_err(&pdev->dev, "Cannot request Blackfin MAC RX IRQ!\n");
 		rc = -EBUSY;
-- 
1.6.3.1

Re: [PATCH 1/4 v2] netdev: bfin_mac: drop useless IRQF_SHARED from Blackfin EMAC interrupt

[David Miller]

From: Mike Frysinger <vapier@gentoo.org>
Date: Fri, 29 May 2009 09:39:45 -0400

> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> The IRQ used by the Blackfin EMAC is internal to the peripheral and cannot
> be used to generate any other interrupt, so there is no point in marking it
> as IRQF_SHARED.
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>

Applied to net-next-2.6

[PATCH 2/5] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[Mike Frysinger]

From: Sonic Zhang <sonic.zhang@analog.com>

Make sure data is really written into the registers before enabling DMA.
Otherwise, the EMAC DMA controller may transfer out a malformed packet.
This patch may also fix netperf bugs or scp bugs.

Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
Signed-off-by: Mike Frysinger <vapier.adi@gmail.com>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
 drivers/net/bfin_mac.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 1905532..626715c 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -688,6 +688,9 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
 		}
 	}
 
+	/* Make sure data is really written into registers before enabling DMA */
+	SSYNC();
+
 	/* enable this packet's dma */
 	current_tx_ptr->desc_a.config |= DMAEN;
 
-- 
1.6.3.1

Re: [PATCH 2/5] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[David Miller]

From: Mike Frysinger <vapier@gentoo.org>
Date: Wed, 27 May 2009 02:10:12 -0400

> From: Sonic Zhang <sonic.zhang@analog.com>
> 
> Make sure data is really written into the registers before enabling DMA.
> Otherwise, the EMAC DMA controller may transfer out a malformed packet.
> This patch may also fix netperf bugs or scp bugs.
> 
> Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
> Signed-off-by: Mike Frysinger <vapier.adi@gmail.com>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>

Instead of using this incredibly non-portable blackfin specific SSYNC
thing, why not read back a register from the device to ensure the
register writes hit the chip just like other drivers do?

If that doesn't work, enhance your commit message to describe (in
detail) why SSYNC is the only way to make this work properly.

I'm not applying this as-is.

Re: [PATCH 2/5] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[Mike Frysinger]

On Fri, May 29, 2009 at 05:04, David Miller wrote:
> From: Mike Frysinger <vapier@gentoo.org>
>> From: Sonic Zhang <sonic.zhang@analog.com>
>>
>> Make sure data is really written into the registers before enabling DMA.
>> Otherwise, the EMAC DMA controller may transfer out a malformed packet.
>> This patch may also fix netperf bugs or scp bugs.
>>
>> Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
>> Signed-off-by: Mike Frysinger <vapier.adi@gmail.com>
>> Signed-off-by: Bryan Wu <cooloney@kernel.org>
>
> Instead of using this incredibly non-portable blackfin specific SSYNC
> thing

that doesnt make sense -- this is a completely non-portable Blackfin
specific driver.  everything in it uses Blackfin macros to access
Blackfin MMRs because this is a device that only exists on Blackfin
chips.

> why not read back a register from the device to ensure the
> register writes hit the chip just like other drivers do?

because that isnt how the Blackfin hardware works in general, and
certainly not how the DMA hardware works.

> If that doesn't work, enhance your commit message to describe (in
> detail) why SSYNC is the only way to make this work properly.

this is how it always is for all Blackfin on-chip registers, and the
hardware manual specifically dictates using SSYNC.
-mike

Re: [PATCH 2/5] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[David Miller]

From: Mike Frysinger <vapier.adi@gmail.com>
Date: Fri, 29 May 2009 06:46:46 -0400

> On Fri, May 29, 2009 at 05:04, David Miller wrote:
>> From: Mike Frysinger <vapier@gentoo.org>
>>> From: Sonic Zhang <sonic.zhang@analog.com>
>>>
>>> Make sure data is really written into the registers before enabling DMA.
>>> Otherwise, the EMAC DMA controller may transfer out a malformed packet.
>>> This patch may also fix netperf bugs or scp bugs.
>>>
>>> Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
>>> Signed-off-by: Mike Frysinger <vapier.adi@gmail.com>
>>> Signed-off-by: Bryan Wu <cooloney@kernel.org>
>>
>> Instead of using this incredibly non-portable blackfin specific SSYNC
>> thing
> 
> that doesnt make sense -- this is a completely non-portable Blackfin
> specific driver.  everything in it uses Blackfin macros to access
> Blackfin MMRs because this is a device that only exists on Blackfin
> chips.

There is never an excuse to not write portable code when you can.

Some day we may have dummy do-nothing bus drivers for every platform
bus or subsystem, so that we can compile any driver on any platform.

Do you want to make that kind of build validation of you code harder?

>> why not read back a register from the device to ensure the
>> register writes hit the chip just like other drivers do?
> 
> because that isnt how the Blackfin hardware works in general, and
> certainly not how the DMA hardware works.

Have you tried reading the register back?  Does it work?

Re: [PATCH 2/5] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[Mike Frysinger]

On Fri, May 29, 2009 at 06:52, David Miller wrote:
> From: Mike Frysinger <vapier.adi@gmail.com>
>> On Fri, May 29, 2009 at 05:04, David Miller wrote:
>>> From: Mike Frysinger <vapier@gentoo.org>
>>>> From: Sonic Zhang <sonic.zhang@analog.com>
>>>>
>>>> Make sure data is really written into the registers before enabling DMA.
>>>> Otherwise, the EMAC DMA controller may transfer out a malformed packet.
>>>> This patch may also fix netperf bugs or scp bugs.
>>>>
>>>> Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
>>>> Signed-off-by: Mike Frysinger <vapier.adi@gmail.com>
>>>> Signed-off-by: Bryan Wu <cooloney@kernel.org>
>>>
>>> Instead of using this incredibly non-portable blackfin specific SSYNC
>>> thing
>>
>> that doesnt make sense -- this is a completely non-portable Blackfin
>> specific driver.  everything in it uses Blackfin macros to access
>> Blackfin MMRs because this is a device that only exists on Blackfin
>> chips.
>
> There is never an excuse to not write portable code when you can.
>
> Some day we may have dummy do-nothing bus drivers for every platform
> bus or subsystem, so that we can compile any driver on any platform.
>
> Do you want to make that kind of build validation of you code harder?

if the portable version is unnecessarily ugly to implement, then yes,
especially this driver which has a lot more issues than an SSYNC macro

>>> why not read back a register from the device to ensure the
>>> register writes hit the chip just like other drivers do?
>>
>> because that isnt how the Blackfin hardware works in general, and
>> certainly not how the DMA hardware works.
>
> Have you tried reading the register back?  Does it work?

that isnt how the DMA is being used here.  there is no register to
read back because the DMA controller is doing the reading indirectly.
but along those lines, it's not entirely clear which DMA registers
this SSYNC is supposed to be protecting because all of the updates are
happening on the descriptors in external memory, not the DMA registers
themselves.  i'll have Sonic clarify.
-mike

Re: [PATCH 2/5] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[David Miller]

From: Mike Frysinger <vapier.adi@gmail.com>
Date: Fri, 29 May 2009 07:20:18 -0400

> that isnt how the DMA is being used here.  there is no register to
> read back because the DMA controller is doing the reading indirectly.
> but along those lines, it's not entirely clear which DMA registers
> this SSYNC is supposed to be protecting because all of the updates are
> happening on the descriptors in external memory, not the DMA registers
> themselves.  i'll have Sonic clarify.

Yes, that's really confusing because the commit message says
"registers".

Re: [PATCH 2/5] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[Mike Frysinger]

On Fri, May 29, 2009 at 07:27, David Miller wrote:
> From: Mike Frysinger <vapier.adi@gmail.com>
>> that isnt how the DMA is being used here.  there is no register to
>> read back because the DMA controller is doing the reading indirectly.
>> but along those lines, it's not entirely clear which DMA registers
>> this SSYNC is supposed to be protecting because all of the updates are
>> happening on the descriptors in external memory, not the DMA registers
>> themselves.  i'll have Sonic clarify.
>
> Yes, that's really confusing because the commit message says
> "registers".

the SSYNC() is still correct, but the comment is wrong/misleading as
it isnt protecting the DMA registers, it's flushing the internal write
buffers
-mike

RE: [PATCH 2/5] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[Zhang, Sonic]

SSYNC here is used to make sure the writing to DMA MMR start_addr is
flushed into the registers before start the DMA controller.

Sonic

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net] 
Sent: Friday, May 29, 2009 7:27 PM
To: vapier.adi@gmail.com
Cc: netdev@vger.kernel.org; uclinux-dist-devel@blackfin.uclinux.org;
Zhang, Sonic; cooloney@kernel.org
Subject: Re: [PATCH 2/5] netdev: bfin_mac: fix malformed UDP packet
transmission when polling with KGDB

From: Mike Frysinger <vapier.adi@gmail.com>
Date: Fri, 29 May 2009 07:20:18 -0400

> that isnt how the DMA is being used here.  there is no register to 
> read back because the DMA controller is doing the reading indirectly.
> but along those lines, it's not entirely clear which DMA registers 
> this SSYNC is supposed to be protecting because all of the updates are

> happening on the descriptors in external memory, not the DMA registers

> themselves.  i'll have Sonic clarify.

Yes, that's really confusing because the commit message says
"registers".

RE: [PATCH 2/5] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[Zhang, Sonic]

It seems the position of the SSYNC should be put to several lines below.

Sonic 

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net] 
Sent: Friday, May 29, 2009 7:27 PM
To: vapier.adi@gmail.com
Cc: netdev@vger.kernel.org; uclinux-dist-devel@blackfin.uclinux.org;
Zhang, Sonic; cooloney@kernel.org
Subject: Re: [PATCH 2/5] netdev: bfin_mac: fix malformed UDP packet
transmission when polling with KGDB

From: Mike Frysinger <vapier.adi@gmail.com>
Date: Fri, 29 May 2009 07:20:18 -0400

> that isnt how the DMA is being used here.  there is no register to 
> read back because the DMA controller is doing the reading indirectly.
> but along those lines, it's not entirely clear which DMA registers 
> this SSYNC is supposed to be protecting because all of the updates are

> happening on the descriptors in external memory, not the DMA registers

> themselves.  i'll have Sonic clarify.

Yes, that's really confusing because the commit message says
"registers".

[PATCH 2/4 v2] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[Mike Frysinger]

From: Sonic Zhang <sonic.zhang@analog.com>

Writes to the DMA descriptors may sit in the internal Blackfin data buffers
and not actually be available when the DMA engine goes to fetch them.  This
does not typically happen, but when dealing with short/fast packets such as
UDP and polling KGDB, this occurs much more frequently.  Same goes for
heavy loads as seen by netperf tests or large scp transfers.  So force the
buffers to drain with SSYNC otherwise we get random malformed packets.

Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
v2
	- make comment/changelog reflect what is actually going on

 drivers/net/bfin_mac.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 1905532..38d34ce 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -688,6 +688,12 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
 		}
 	}
 
+	/* make sure the internal data buffers in the core are drained
+	 * so that the DMA descriptors are completely written when the
+	 * DMA engine goes to fetch them below
+	 */
+	SSYNC();
+
 	/* enable this packet's dma */
 	current_tx_ptr->desc_a.config |= DMAEN;
 
-- 
1.6.3.1

Re: [PATCH 2/4 v2] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[David Miller]

From: Mike Frysinger <vapier@gentoo.org>
Date: Fri, 29 May 2009 09:40:43 -0400

> From: Sonic Zhang <sonic.zhang@analog.com>
> 
> Writes to the DMA descriptors may sit in the internal Blackfin data buffers
> and not actually be available when the DMA engine goes to fetch them.  This
> does not typically happen, but when dealing with short/fast packets such as
> UDP and polling KGDB, this occurs much more frequently.  Same goes for
> heavy loads as seen by netperf tests or large scp transfers.  So force the
> buffers to drain with SSYNC otherwise we get random malformed packets.
> 
> Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>

Applied to net-next-2.6

RE: [PATCH 2/4 v2] netdev: bfin_mac: fix malformed UDP packet transmission when polling with KGDB

[Zhang, Sonic]

OK. Mike's explanation is correct.

Sonic 

-----Original Message-----
From: Mike Frysinger [mailto:vapier@gentoo.org] 
Sent: Friday, May 29, 2009 9:41 PM
To: netdev@vger.kernel.org
Cc: uclinux-dist-devel@blackfin.uclinux.org; Zhang, Sonic; Bryan Wu
Subject: [PATCH 2/4 v2] netdev: bfin_mac: fix malformed UDP packet
transmission when polling with KGDB

From: Sonic Zhang <sonic.zhang@analog.com>

Writes to the DMA descriptors may sit in the internal Blackfin data
buffers and not actually be available when the DMA engine goes to fetch
them.  This does not typically happen, but when dealing with short/fast
packets such as UDP and polling KGDB, this occurs much more frequently.
Same goes for heavy loads as seen by netperf tests or large scp
transfers.  So force the buffers to drain with SSYNC otherwise we get
random malformed packets.

Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
v2
	- make comment/changelog reflect what is actually going on

 drivers/net/bfin_mac.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c index
1905532..38d34ce 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -688,6 +688,12 @@ static int bfin_mac_hard_start_xmit(struct sk_buff
*skb,
 		}
 	}
 
+	/* make sure the internal data buffers in the core are drained
+	 * so that the DMA descriptors are completely written when the
+	 * DMA engine goes to fetch them below
+	 */
+	SSYNC();
+
 	/* enable this packet's dma */
 	current_tx_ptr->desc_a.config |= DMAEN;
 
--
1.6.3.1

[PATCH 3/5] netdev: bfin_mac: fix performance issue found by netperf

[Mike Frysinger]

From: Michael Hennerich <michael.hennerich@analog.com>

- Remove dead long delay
- Use proper defines
- Remove broken implementation of the TX DMA Data Alignment TXDWA feature

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
 drivers/net/bfin_mac.c |  110 +++++++++++------------------------------------
 1 files changed, 26 insertions(+), 84 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 626715c..1d63e3a 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -194,13 +194,13 @@ static int desc_list_init(void)
 		struct dma_descriptor *b = &(r->desc_b);
 
 		/* allocate a new skb for next time receive */
-		new_skb = dev_alloc_skb(PKT_BUF_SZ + 2);
+		new_skb = dev_alloc_skb(PKT_BUF_SZ + NET_IP_ALIGN);
 		if (!new_skb) {
 			printk(KERN_NOTICE DRV_NAME
 			       ": init: low on mem - packet dropped\n");
 			goto init_error;
 		}
-		skb_reserve(new_skb, 2);
+		skb_reserve(new_skb, NET_IP_ALIGN);
 		r->skb = new_skb;
 
 		/*
@@ -566,9 +566,9 @@ static void adjust_tx_list(void)
 	 */
 	if (current_tx_ptr->next->next == tx_list_head) {
 		while (tx_list_head->status.status_word == 0) {
-			mdelay(1);
+			udelay(10);
 			if (tx_list_head->status.status_word != 0
-			    || !(bfin_read_DMA2_IRQ_STATUS() & 0x08)) {
+			    || !(bfin_read_DMA2_IRQ_STATUS() & DMA_RUN)) {
 				goto adjust_head;
 			}
 			if (timeout_cnt-- < 0) {
@@ -606,86 +606,28 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
 				struct net_device *dev)
 {
 	u16 *data;
-
+	u32 data_align = (u32)(skb->data) & 0x3;
 	current_tx_ptr->skb = skb;
 
-	if (ANOMALY_05000285) {
-		/*
-		 * TXDWA feature is not avaible to older revision < 0.3 silicon
-		 * of BF537
-		 *
-		 * Only if data buffer is ODD WORD alignment, we do not
-		 * need to memcpy
-		 */
-		u32 data_align = (u32)(skb->data) & 0x3;
-		if (data_align == 0x2) {
-			/* move skb->data to current_tx_ptr payload */
-			data = (u16 *)(skb->data) - 1;
-			*data = (u16)(skb->len);
-			current_tx_ptr->desc_a.start_addr = (u32)data;
-			/* this is important! */
-			blackfin_dcache_flush_range((u32)data,
-					(u32)((u8 *)data + skb->len + 4));
-		} else {
-			*((u16 *)(current_tx_ptr->packet)) = (u16)(skb->len);
-			memcpy((u8 *)(current_tx_ptr->packet + 2), skb->data,
-				skb->len);
-			current_tx_ptr->desc_a.start_addr =
-				(u32)current_tx_ptr->packet;
-			if (current_tx_ptr->status.status_word != 0)
-				current_tx_ptr->status.status_word = 0;
-			blackfin_dcache_flush_range(
-				(u32)current_tx_ptr->packet,
-				(u32)(current_tx_ptr->packet + skb->len + 2));
-		}
+	if (data_align == 0x2) {
+		/* move skb->data to current_tx_ptr payload */
+		data = (u16 *)(skb->data) - 1;
+				*data = (u16)(skb->len);
+		current_tx_ptr->desc_a.start_addr = (u32)data;
+		/* this is important! */
+		blackfin_dcache_flush_range((u32)data,
+				(u32)((u8 *)data + skb->len + 4));
 	} else {
-		/*
-		 * TXDWA feature is avaible to revision < 0.3 silicon of
-		 * BF537 and always avaible to BF52x
-		 */
-		u32 data_align = (u32)(skb->data) & 0x3;
-		if (data_align == 0x0) {
-			u16 sysctl = bfin_read_EMAC_SYSCTL();
-			sysctl |= TXDWA;
-			bfin_write_EMAC_SYSCTL(sysctl);
-
-			/* move skb->data to current_tx_ptr payload */
-			data = (u16 *)(skb->data) - 2;
-			*data = (u16)(skb->len);
-			current_tx_ptr->desc_a.start_addr = (u32)data;
-			/* this is important! */
-			blackfin_dcache_flush_range(
-					(u32)data,
-					(u32)((u8 *)data + skb->len + 4));
-		} else if (data_align == 0x2) {
-			u16 sysctl = bfin_read_EMAC_SYSCTL();
-			sysctl &= ~TXDWA;
-			bfin_write_EMAC_SYSCTL(sysctl);
-
-			/* move skb->data to current_tx_ptr payload */
-			data = (u16 *)(skb->data) - 1;
-			*data = (u16)(skb->len);
-			current_tx_ptr->desc_a.start_addr = (u32)data;
-			/* this is important! */
-			blackfin_dcache_flush_range(
-					(u32)data,
-					(u32)((u8 *)data + skb->len + 4));
-		} else {
-			u16 sysctl = bfin_read_EMAC_SYSCTL();
-			sysctl &= ~TXDWA;
-			bfin_write_EMAC_SYSCTL(sysctl);
-
-			*((u16 *)(current_tx_ptr->packet)) = (u16)(skb->len);
-			memcpy((u8 *)(current_tx_ptr->packet + 2), skb->data,
-				skb->len);
-			current_tx_ptr->desc_a.start_addr =
-				(u32)current_tx_ptr->packet;
-			if (current_tx_ptr->status.status_word != 0)
-				current_tx_ptr->status.status_word = 0;
-			blackfin_dcache_flush_range(
-				(u32)current_tx_ptr->packet,
-				(u32)(current_tx_ptr->packet + skb->len + 2));
-		}
+		*((u16 *)(current_tx_ptr->packet)) = (u16)(skb->len);
+		memcpy((u8 *)(current_tx_ptr->packet + 2), skb->data,
+			skb->len);
+		current_tx_ptr->desc_a.start_addr =
+			(u32)current_tx_ptr->packet;
+		if (current_tx_ptr->status.status_word != 0)
+			current_tx_ptr->status.status_word = 0;
+		blackfin_dcache_flush_range(
+			(u32)current_tx_ptr->packet,
+			(u32)(current_tx_ptr->packet + skb->len + 2));
 	}
 
 	/* Make sure data is really written into registers before enabling DMA */
@@ -695,7 +637,7 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
 	current_tx_ptr->desc_a.config |= DMAEN;
 
 	/* tx dma is running, just return */
-	if (bfin_read_DMA2_IRQ_STATUS() & 0x08)
+	if (bfin_read_DMA2_IRQ_STATUS() & DMA_RUN)
 		goto out;
 
 	/* tx dma is not running */
@@ -721,7 +663,7 @@ static void bfin_mac_rx(struct net_device *dev)
 
 	/* allocate a new skb for next time receive */
 	skb = current_rx_ptr->skb;
-	new_skb = dev_alloc_skb(PKT_BUF_SZ + 2);
+	new_skb = dev_alloc_skb(PKT_BUF_SZ + NET_IP_ALIGN);
 	if (!new_skb) {
 		printk(KERN_NOTICE DRV_NAME
 		       ": rx: low on mem - packet dropped\n");
@@ -729,7 +671,7 @@ static void bfin_mac_rx(struct net_device *dev)
 		goto out;
 	}
 	/* reserve 2 bytes for RXDWA padding */
-	skb_reserve(new_skb, 2);
+	skb_reserve(new_skb, NET_IP_ALIGN);
 	current_rx_ptr->skb = new_skb;
 	current_rx_ptr->desc_a.start_addr = (unsigned long)new_skb->data - 2;
 
-- 
1.6.3.1

Re: [PATCH 3/5] netdev: bfin_mac: fix performance issue found by netperf

[David Miller]

From: Mike Frysinger <vapier@gentoo.org>
Date: Wed, 27 May 2009 02:10:13 -0400

> @@ -606,86 +606,28 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
>  				struct net_device *dev)
>  {
>  	u16 *data;
> -
> +	u32 data_align = (u32)(skb->data) & 0x3;

A pointer is not castable directly to a u32 everywhere.  It can be
64-bits and in which case GCC will warn here.

I know this is a blackfin specific driver, but please write portable
code.

Use "unsigned long" or similar.

I'm not applying this as it is now.

[PATCH 3/4 v2] netdev: bfin_mac: fix performance issue found by netperf

[Mike Frysinger]

From: Michael Hennerich <michael.hennerich@analog.com>

- Remove dead long delay
- Use proper defines
- Remove broken implementation of the TX DMA Data Alignment TXDWA feature

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
v2
	- update cast based on David Miller's feedback

 drivers/net/bfin_mac.c |  110 +++++++++++------------------------------------
 1 files changed, 26 insertions(+), 84 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 38d34ce..f0f1eb9 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -194,13 +194,13 @@ static int desc_list_init(void)
 		struct dma_descriptor *b = &(r->desc_b);
 
 		/* allocate a new skb for next time receive */
-		new_skb = dev_alloc_skb(PKT_BUF_SZ + 2);
+		new_skb = dev_alloc_skb(PKT_BUF_SZ + NET_IP_ALIGN);
 		if (!new_skb) {
 			printk(KERN_NOTICE DRV_NAME
 			       ": init: low on mem - packet dropped\n");
 			goto init_error;
 		}
-		skb_reserve(new_skb, 2);
+		skb_reserve(new_skb, NET_IP_ALIGN);
 		r->skb = new_skb;
 
 		/*
@@ -566,9 +566,9 @@ static void adjust_tx_list(void)
 	 */
 	if (current_tx_ptr->next->next == tx_list_head) {
 		while (tx_list_head->status.status_word == 0) {
-			mdelay(1);
+			udelay(10);
 			if (tx_list_head->status.status_word != 0
-			    || !(bfin_read_DMA2_IRQ_STATUS() & 0x08)) {
+			    || !(bfin_read_DMA2_IRQ_STATUS() & DMA_RUN)) {
 				goto adjust_head;
 			}
 			if (timeout_cnt-- < 0) {
@@ -606,86 +606,28 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
 				struct net_device *dev)
 {
 	u16 *data;
-
+	u32 data_align = (unsigned long)(skb->data) & 0x3;
 	current_tx_ptr->skb = skb;
 
-	if (ANOMALY_05000285) {
-		/*
-		 * TXDWA feature is not avaible to older revision < 0.3 silicon
-		 * of BF537
-		 *
-		 * Only if data buffer is ODD WORD alignment, we do not
-		 * need to memcpy
-		 */
-		u32 data_align = (u32)(skb->data) & 0x3;
-		if (data_align == 0x2) {
-			/* move skb->data to current_tx_ptr payload */
-			data = (u16 *)(skb->data) - 1;
-			*data = (u16)(skb->len);
-			current_tx_ptr->desc_a.start_addr = (u32)data;
-			/* this is important! */
-			blackfin_dcache_flush_range((u32)data,
-					(u32)((u8 *)data + skb->len + 4));
-		} else {
-			*((u16 *)(current_tx_ptr->packet)) = (u16)(skb->len);
-			memcpy((u8 *)(current_tx_ptr->packet + 2), skb->data,
-				skb->len);
-			current_tx_ptr->desc_a.start_addr =
-				(u32)current_tx_ptr->packet;
-			if (current_tx_ptr->status.status_word != 0)
-				current_tx_ptr->status.status_word = 0;
-			blackfin_dcache_flush_range(
-				(u32)current_tx_ptr->packet,
-				(u32)(current_tx_ptr->packet + skb->len + 2));
-		}
+	if (data_align == 0x2) {
+		/* move skb->data to current_tx_ptr payload */
+		data = (u16 *)(skb->data) - 1;
+				*data = (u16)(skb->len);
+		current_tx_ptr->desc_a.start_addr = (u32)data;
+		/* this is important! */
+		blackfin_dcache_flush_range((u32)data,
+				(u32)((u8 *)data + skb->len + 4));
 	} else {
-		/*
-		 * TXDWA feature is avaible to revision < 0.3 silicon of
-		 * BF537 and always avaible to BF52x
-		 */
-		u32 data_align = (u32)(skb->data) & 0x3;
-		if (data_align == 0x0) {
-			u16 sysctl = bfin_read_EMAC_SYSCTL();
-			sysctl |= TXDWA;
-			bfin_write_EMAC_SYSCTL(sysctl);
-
-			/* move skb->data to current_tx_ptr payload */
-			data = (u16 *)(skb->data) - 2;
-			*data = (u16)(skb->len);
-			current_tx_ptr->desc_a.start_addr = (u32)data;
-			/* this is important! */
-			blackfin_dcache_flush_range(
-					(u32)data,
-					(u32)((u8 *)data + skb->len + 4));
-		} else if (data_align == 0x2) {
-			u16 sysctl = bfin_read_EMAC_SYSCTL();
-			sysctl &= ~TXDWA;
-			bfin_write_EMAC_SYSCTL(sysctl);
-
-			/* move skb->data to current_tx_ptr payload */
-			data = (u16 *)(skb->data) - 1;
-			*data = (u16)(skb->len);
-			current_tx_ptr->desc_a.start_addr = (u32)data;
-			/* this is important! */
-			blackfin_dcache_flush_range(
-					(u32)data,
-					(u32)((u8 *)data + skb->len + 4));
-		} else {
-			u16 sysctl = bfin_read_EMAC_SYSCTL();
-			sysctl &= ~TXDWA;
-			bfin_write_EMAC_SYSCTL(sysctl);
-
-			*((u16 *)(current_tx_ptr->packet)) = (u16)(skb->len);
-			memcpy((u8 *)(current_tx_ptr->packet + 2), skb->data,
-				skb->len);
-			current_tx_ptr->desc_a.start_addr =
-				(u32)current_tx_ptr->packet;
-			if (current_tx_ptr->status.status_word != 0)
-				current_tx_ptr->status.status_word = 0;
-			blackfin_dcache_flush_range(
-				(u32)current_tx_ptr->packet,
-				(u32)(current_tx_ptr->packet + skb->len + 2));
-		}
+		*((u16 *)(current_tx_ptr->packet)) = (u16)(skb->len);
+		memcpy((u8 *)(current_tx_ptr->packet + 2), skb->data,
+			skb->len);
+		current_tx_ptr->desc_a.start_addr =
+			(u32)current_tx_ptr->packet;
+		if (current_tx_ptr->status.status_word != 0)
+			current_tx_ptr->status.status_word = 0;
+		blackfin_dcache_flush_range(
+			(u32)current_tx_ptr->packet,
+			(u32)(current_tx_ptr->packet + skb->len + 2));
 	}
 
 	/* make sure the internal data buffers in the core are drained
@@ -698,7 +640,7 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
 	current_tx_ptr->desc_a.config |= DMAEN;
 
 	/* tx dma is running, just return */
-	if (bfin_read_DMA2_IRQ_STATUS() & 0x08)
+	if (bfin_read_DMA2_IRQ_STATUS() & DMA_RUN)
 		goto out;
 
 	/* tx dma is not running */
@@ -724,7 +666,7 @@ static void bfin_mac_rx(struct net_device *dev)
 
 	/* allocate a new skb for next time receive */
 	skb = current_rx_ptr->skb;
-	new_skb = dev_alloc_skb(PKT_BUF_SZ + 2);
+	new_skb = dev_alloc_skb(PKT_BUF_SZ + NET_IP_ALIGN);
 	if (!new_skb) {
 		printk(KERN_NOTICE DRV_NAME
 		       ": rx: low on mem - packet dropped\n");
@@ -732,7 +674,7 @@ static void bfin_mac_rx(struct net_device *dev)
 		goto out;
 	}
 	/* reserve 2 bytes for RXDWA padding */
-	skb_reserve(new_skb, 2);
+	skb_reserve(new_skb, NET_IP_ALIGN);
 	current_rx_ptr->skb = new_skb;
 	current_rx_ptr->desc_a.start_addr = (unsigned long)new_skb->data - 2;
 
-- 
1.6.3.1

Re: [PATCH 3/4 v2] netdev: bfin_mac: fix performance issue found by netperf

[David Miller]

From: Mike Frysinger <vapier@gentoo.org>
Date: Fri, 29 May 2009 09:41:15 -0400

> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> - Remove dead long delay
> - Use proper defines
> - Remove broken implementation of the TX DMA Data Alignment TXDWA feature
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>

Applied to net-next-2.6

[PATCH 4/5] netdev: bfin_mac: add Blackfin MII bus to platform bus to allow DSA access

[Mike Frysinger]

From: Graf Yang <graf.yang@analog.com>

When we register the MII bus to the platfrom bus, the Distributed Switch
Architecture can hook in transparently.

Signed-off-by: Graf Yang <graf.yang@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/net/bfin_mac.c |  114 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 83 insertions(+), 31 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 1d63e3a..7f8ea0f 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -968,7 +968,8 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
 {
 	struct net_device *ndev;
 	struct bfin_mac_local *lp;
-	int rc, i;
+	struct platform_device *pd;
+	int rc;
 
 	ndev = alloc_etherdev(sizeof(struct bfin_mac_local));
 	if (!ndev) {
@@ -993,13 +994,6 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
 		goto out_err_probe_mac;
 	}
 
-	/* set the GPIO pins to Ethernet mode */
-	rc = peripheral_request_list(pin_req, DRV_NAME);
-	if (rc) {
-		dev_err(&pdev->dev, "Requesting peripherals failed!\n");
-		rc = -EFAULT;
-		goto out_err_setup_pin_mux;
-	}
 
 	/*
 	 * Is it valid? (Did bootloader initialize it?)
@@ -1015,26 +1009,14 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
 
 	setup_mac_addr(ndev->dev_addr);
 
-	/* MDIO bus initial */
-	lp->mii_bus = mdiobus_alloc();
-	if (lp->mii_bus == NULL)
-		goto out_err_mdiobus_alloc;
-
-	lp->mii_bus->priv = ndev;
-	lp->mii_bus->read = bfin_mdiobus_read;
-	lp->mii_bus->write = bfin_mdiobus_write;
-	lp->mii_bus->reset = bfin_mdiobus_reset;
-	lp->mii_bus->name = "bfin_mac_mdio";
-	snprintf(lp->mii_bus->id, MII_BUS_ID_SIZE, "0");
-	lp->mii_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
-	for (i = 0; i < PHY_MAX_ADDR; ++i)
-		lp->mii_bus->irq[i] = PHY_POLL;
-
-	rc = mdiobus_register(lp->mii_bus);
-	if (rc) {
-		dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
-		goto out_err_mdiobus_register;
+	if (!pdev->dev.platform_data) {
+		dev_err(&pdev->dev, "Cannot get platform device bfin_mii_bus!\n");
+		rc = -ENODEV;
+		goto out_err_probe_mac;
 	}
+	pd = pdev->dev.platform_data;
+	lp->mii_bus = platform_get_drvdata(pd);
+	lp->mii_bus->priv = ndev;
 
 	rc = mii_probe(ndev);
 	if (rc) {
@@ -1076,11 +1058,8 @@ out_err_reg_ndev:
 out_err_request_irq:
 out_err_mii_probe:
 	mdiobus_unregister(lp->mii_bus);
-out_err_mdiobus_register:
 	mdiobus_free(lp->mii_bus);
-out_err_mdiobus_alloc:
 	peripheral_free_list(pin_req);
-out_err_setup_pin_mux:
 out_err_probe_mac:
 	platform_set_drvdata(pdev, NULL);
 	free_netdev(ndev);
@@ -1134,6 +1113,74 @@ static int bfin_mac_resume(struct platform_device *pdev)
 #define bfin_mac_resume NULL
 #endif	/* CONFIG_PM */
 
+static int __devinit bfin_mii_bus_probe(struct platform_device *pdev)
+{
+	struct mii_bus *miibus;
+	int rc, i;
+
+	/*
+	 * We are setting up a network card,
+	 * so set the GPIO pins to Ethernet mode
+	 */
+	rc = peripheral_request_list(pin_req, DRV_NAME);
+	if (rc) {
+		dev_err(&pdev->dev, "Requesting peripherals failed!\n");
+		return rc;
+	}
+
+	rc = -ENOMEM;
+	miibus = mdiobus_alloc();
+	if (miibus == NULL)
+		goto out_err_alloc;
+	miibus->read = bfin_mdiobus_read;
+	miibus->write = bfin_mdiobus_write;
+	miibus->reset = bfin_mdiobus_reset;
+
+	miibus->parent = &pdev->dev;
+	miibus->name = "bfin_mii_bus";
+	snprintf(miibus->id, MII_BUS_ID_SIZE, "0");
+	miibus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
+	if (miibus->irq == NULL)
+		goto out_err_alloc;
+	for (i = 0; i < PHY_MAX_ADDR; ++i)
+		miibus->irq[i] = PHY_POLL;
+
+	rc = mdiobus_register(miibus);
+	if (rc) {
+		dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
+		goto out_err_mdiobus_register;
+	}
+
+	platform_set_drvdata(pdev, miibus);
+	return 0;
+
+out_err_mdiobus_register:
+	mdiobus_free(miibus);
+out_err_alloc:
+	peripheral_free_list(pin_req);
+
+	return rc;
+}
+
+static int __devexit bfin_mii_bus_remove(struct platform_device *pdev)
+{
+	struct mii_bus *miibus = platform_get_drvdata(pdev);
+	platform_set_drvdata(pdev, NULL);
+	mdiobus_unregister(miibus);
+	mdiobus_free(miibus);
+	peripheral_free_list(pin_req);
+	return 0;
+}
+
+static struct platform_driver bfin_mii_bus_driver = {
+	.probe = bfin_mii_bus_probe,
+	.remove = __devexit_p(bfin_mii_bus_remove),
+	.driver = {
+		.name = "bfin_mii_bus",
+		.owner	= THIS_MODULE,
+	},
+};
+
 static struct platform_driver bfin_mac_driver = {
 	.probe = bfin_mac_probe,
 	.remove = __devexit_p(bfin_mac_remove),
@@ -1147,7 +1194,11 @@ static struct platform_driver bfin_mac_driver = {
 
 static int __init bfin_mac_init(void)
 {
-	return platform_driver_register(&bfin_mac_driver);
+	int ret;
+	ret = platform_driver_register(&bfin_mii_bus_driver);
+	if (!ret)
+		return platform_driver_register(&bfin_mac_driver);
+	return -ENODEV;
 }
 
 module_init(bfin_mac_init);
@@ -1155,6 +1206,7 @@ module_init(bfin_mac_init);
 static void __exit bfin_mac_cleanup(void)
 {
 	platform_driver_unregister(&bfin_mac_driver);
+	platform_driver_unregister(&bfin_mii_bus_driver);
 }
 
 module_exit(bfin_mac_cleanup);
-- 
1.6.3.1

Re: [PATCH 4/5] netdev: bfin_mac: add Blackfin MII bus to platform bus to allow DSA access

[David Miller]

From: Mike Frysinger <vapier@gentoo.org>
Date: Wed, 27 May 2009 02:10:14 -0400

> From: Graf Yang <graf.yang@analog.com>
> 
> When we register the MII bus to the platfrom bus, the Distributed Switch
> Architecture can hook in transparently.
> 
> Signed-off-by: Graf Yang <graf.yang@analog.com>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Applied to net-next-2.6

[PATCH 5/5] netdev: bfin_mac: fix crash when unloading module

[Mike Frysinger]

From: Graf Yang <graf.yang@analog.com>

If we build the Blackfin EMAC driver as a module and then unload it at
runtime, it would cause a crash:
root:/> rmmod bfin_mac
Hardware Trace:
   0 Target : <0x00004624> { _dump_bfin_trace_buffer + 0x0 }
     Source : <0x000a57b2> { _mdiobus_unregister + 0x42 } CALL pcrel
   1 Target : <0x000a57b2> { _mdiobus_unregister + 0x42 }
     Source : <0x000a577c> { _mdiobus_unregister + 0xc } IF !CC JUMP
   2 Target : <0x000a5770> { _mdiobus_unregister + 0x0 }
     Source : <0x037a4c9e> { :bfin_mac:_cleanup_module + 0x4a } CALL (P2)
   3 Target : <0x037a4c80> { :bfin_mac:_cleanup_module + 0x2c }
     Source : <0x000a0a2e> { _platform_drv_remove + 0xe } JUMP (P1)
   4 Target : <0x000a0a20> { _platform_drv_remove + 0x0 }
     Source : <0x0009fcc0> { ___device_release_driver + 0x50 } CALL (P2)
   ....

URL: http://blackfin.uclinux.org/gf/tracker/4885
Signed-off-by: Graf Yang <graf.yang@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/net/bfin_mac.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 7f8ea0f..e764f8c 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -1074,8 +1074,7 @@ static int __devexit bfin_mac_remove(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, NULL);
 
-	mdiobus_unregister(lp->mii_bus);
-	mdiobus_free(lp->mii_bus);
+	lp->mii_bus->priv = NULL;
 
 	unregister_netdev(ndev);
 
-- 
1.6.3.1

Re: [PATCH 5/5] netdev: bfin_mac: fix crash when unloading module

[David Miller]

From: Mike Frysinger <vapier@gentoo.org>
Date: Wed, 27 May 2009 02:10:15 -0400

> If we build the Blackfin EMAC driver as a module and then unload it at
> runtime, it would cause a crash:
> root:/> rmmod bfin_mac

Actually, I reverted patch #4.

This bug is caused by patch #4, integrate this bug fix into
patch #4 and resubmit as one whole change.

Otherwise the tree is not properly bisectable.

Thank you.

Re: [Uclinux-dist-devel] [PATCH 5/5] netdev: bfin_mac: fix crash when unloading module

[Mike Frysinger]

On Fri, May 29, 2009 at 05:10, David Miller wrote:
> From: Mike Frysinger <vapier@gentoo.org>
>> If we build the Blackfin EMAC driver as a module and then unload it at
>> runtime, it would cause a crash:
>> root:/> rmmod bfin_mac
>
> Actually, I reverted patch #4.
>
> This bug is caused by patch #4, integrate this bug fix into
> patch #4 and resubmit as one whole change.

will do, thanks
-mike

[PATCH 4/4 v2] netdev: bfin_mac: add Blackfin MII bus to platform bus to allow DSA access

[Mike Frysinger]

From: Graf Yang <graf.yang@analog.com>

When we register the MII bus to the platfrom bus, the Distributed Switch
Architecture can hook in transparently.

Signed-off-by: Graf Yang <graf.yang@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- squash patches 4 & 5 in previous series

 drivers/net/bfin_mac.c |  117 ++++++++++++++++++++++++++++++++++-------------
 1 files changed, 84 insertions(+), 33 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index f0f1eb9..955a469 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -971,7 +971,8 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
 {
 	struct net_device *ndev;
 	struct bfin_mac_local *lp;
-	int rc, i;
+	struct platform_device *pd;
+	int rc;
 
 	ndev = alloc_etherdev(sizeof(struct bfin_mac_local));
 	if (!ndev) {
@@ -996,13 +997,6 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
 		goto out_err_probe_mac;
 	}
 
-	/* set the GPIO pins to Ethernet mode */
-	rc = peripheral_request_list(pin_req, DRV_NAME);
-	if (rc) {
-		dev_err(&pdev->dev, "Requesting peripherals failed!\n");
-		rc = -EFAULT;
-		goto out_err_setup_pin_mux;
-	}
 
 	/*
 	 * Is it valid? (Did bootloader initialize it?)
@@ -1018,26 +1012,14 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
 
 	setup_mac_addr(ndev->dev_addr);
 
-	/* MDIO bus initial */
-	lp->mii_bus = mdiobus_alloc();
-	if (lp->mii_bus == NULL)
-		goto out_err_mdiobus_alloc;
-
-	lp->mii_bus->priv = ndev;
-	lp->mii_bus->read = bfin_mdiobus_read;
-	lp->mii_bus->write = bfin_mdiobus_write;
-	lp->mii_bus->reset = bfin_mdiobus_reset;
-	lp->mii_bus->name = "bfin_mac_mdio";
-	snprintf(lp->mii_bus->id, MII_BUS_ID_SIZE, "0");
-	lp->mii_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
-	for (i = 0; i < PHY_MAX_ADDR; ++i)
-		lp->mii_bus->irq[i] = PHY_POLL;
-
-	rc = mdiobus_register(lp->mii_bus);
-	if (rc) {
-		dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
-		goto out_err_mdiobus_register;
+	if (!pdev->dev.platform_data) {
+		dev_err(&pdev->dev, "Cannot get platform device bfin_mii_bus!\n");
+		rc = -ENODEV;
+		goto out_err_probe_mac;
 	}
+	pd = pdev->dev.platform_data;
+	lp->mii_bus = platform_get_drvdata(pd);
+	lp->mii_bus->priv = ndev;
 
 	rc = mii_probe(ndev);
 	if (rc) {
@@ -1079,11 +1061,8 @@ out_err_reg_ndev:
 out_err_request_irq:
 out_err_mii_probe:
 	mdiobus_unregister(lp->mii_bus);
-out_err_mdiobus_register:
 	mdiobus_free(lp->mii_bus);
-out_err_mdiobus_alloc:
 	peripheral_free_list(pin_req);
-out_err_setup_pin_mux:
 out_err_probe_mac:
 	platform_set_drvdata(pdev, NULL);
 	free_netdev(ndev);
@@ -1098,8 +1077,7 @@ static int __devexit bfin_mac_remove(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, NULL);
 
-	mdiobus_unregister(lp->mii_bus);
-	mdiobus_free(lp->mii_bus);
+	lp->mii_bus->priv = NULL;
 
 	unregister_netdev(ndev);
 
@@ -1137,6 +1115,74 @@ static int bfin_mac_resume(struct platform_device *pdev)
 #define bfin_mac_resume NULL
 #endif	/* CONFIG_PM */
 
+static int __devinit bfin_mii_bus_probe(struct platform_device *pdev)
+{
+	struct mii_bus *miibus;
+	int rc, i;
+
+	/*
+	 * We are setting up a network card,
+	 * so set the GPIO pins to Ethernet mode
+	 */
+	rc = peripheral_request_list(pin_req, DRV_NAME);
+	if (rc) {
+		dev_err(&pdev->dev, "Requesting peripherals failed!\n");
+		return rc;
+	}
+
+	rc = -ENOMEM;
+	miibus = mdiobus_alloc();
+	if (miibus == NULL)
+		goto out_err_alloc;
+	miibus->read = bfin_mdiobus_read;
+	miibus->write = bfin_mdiobus_write;
+	miibus->reset = bfin_mdiobus_reset;
+
+	miibus->parent = &pdev->dev;
+	miibus->name = "bfin_mii_bus";
+	snprintf(miibus->id, MII_BUS_ID_SIZE, "0");
+	miibus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
+	if (miibus->irq == NULL)
+		goto out_err_alloc;
+	for (i = 0; i < PHY_MAX_ADDR; ++i)
+		miibus->irq[i] = PHY_POLL;
+
+	rc = mdiobus_register(miibus);
+	if (rc) {
+		dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
+		goto out_err_mdiobus_register;
+	}
+
+	platform_set_drvdata(pdev, miibus);
+	return 0;
+
+out_err_mdiobus_register:
+	mdiobus_free(miibus);
+out_err_alloc:
+	peripheral_free_list(pin_req);
+
+	return rc;
+}
+
+static int __devexit bfin_mii_bus_remove(struct platform_device *pdev)
+{
+	struct mii_bus *miibus = platform_get_drvdata(pdev);
+	platform_set_drvdata(pdev, NULL);
+	mdiobus_unregister(miibus);
+	mdiobus_free(miibus);
+	peripheral_free_list(pin_req);
+	return 0;
+}
+
+static struct platform_driver bfin_mii_bus_driver = {
+	.probe = bfin_mii_bus_probe,
+	.remove = __devexit_p(bfin_mii_bus_remove),
+	.driver = {
+		.name = "bfin_mii_bus",
+		.owner	= THIS_MODULE,
+	},
+};
+
 static struct platform_driver bfin_mac_driver = {
 	.probe = bfin_mac_probe,
 	.remove = __devexit_p(bfin_mac_remove),
@@ -1150,7 +1196,11 @@ static struct platform_driver bfin_mac_driver = {
 
 static int __init bfin_mac_init(void)
 {
-	return platform_driver_register(&bfin_mac_driver);
+	int ret;
+	ret = platform_driver_register(&bfin_mii_bus_driver);
+	if (!ret)
+		return platform_driver_register(&bfin_mac_driver);
+	return -ENODEV;
 }
 
 module_init(bfin_mac_init);
@@ -1158,6 +1208,7 @@ module_init(bfin_mac_init);
 static void __exit bfin_mac_cleanup(void)
 {
 	platform_driver_unregister(&bfin_mac_driver);
+	platform_driver_unregister(&bfin_mii_bus_driver);
 }
 
 module_exit(bfin_mac_cleanup);
-- 
1.6.3.1

Re: [PATCH 4/4 v2] netdev: bfin_mac: add Blackfin MII bus to platform bus to allow DSA access

[David Miller]

From: Mike Frysinger <vapier@gentoo.org>
Date: Fri, 29 May 2009 09:41:48 -0400

> From: Graf Yang <graf.yang@analog.com>
> 
> When we register the MII bus to the platfrom bus, the Distributed Switch
> Architecture can hook in transparently.
> 
> Signed-off-by: Graf Yang <graf.yang@analog.com>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Applied to net-next-2.6