[PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Stefan Roese]

From: Pravin M. Bathija <pbathija@amcc.com>

Problem Description and Fix
---------------------------
When a pause packet(with destination as reserved Multicast address) is
received by the EMAC hardware to control the flow of frames being
transmitted by it, it is dropped by the hardware unless the reserved
Multicast address is hashed in to the GAHT[1-4] registers. This code fix
adds the default reserved multicast address to the GAHT[1-4] registers
in the EMAC(s) present on the chip. The flow control with Pause packets
will only work if the following register bits are programmed in EMAC:
EMACx_MR1[APP] = 1
EMACx_RMR[BAE] = 1
EMACx_RMR[MAE] = 1

Behavior that may be observed in a running system
-------------------------------------------------
A host transferring data from a PPC based system may send a Pause packet
to the PPC EMAC requesting it to slow down the flow of packets. If the
default reserved multicast MAC address is not programmed into the
GAHT[1-4] registers this Pause packet will be dropped by PPC EMAC and no
Flow Control will be done.

Signed-off-by: Pravin M. Bathija <pbathija@amcc.com>
Signed-off-by: Stefan Roese <sr@denx.de>
---
 drivers/net/ibm_newemac/core.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ibm_newemac/core.c b/drivers/net/ibm_newemac/core.c
index e6c69f7..0789802 100644
--- a/drivers/net/ibm_newemac/core.c
+++ b/drivers/net/ibm_newemac/core.c
@@ -143,6 +143,10 @@ static inline void emac_report_timeout_error(struct emac_instance *dev,
 #define STOP_TIMEOUT_1000	13
 #define STOP_TIMEOUT_1000_JUMBO	73
 
+static unsigned char default_mcast_addr[] = {
+	0x01, 0x80, 0xC2, 0x00, 0x00, 0x01
+};
+
 /* Please, keep in sync with struct ibm_emac_stats/ibm_emac_error_stats */
 static const char emac_stats_keys[EMAC_ETHTOOL_STATS_COUNT][ETH_GSTRING_LEN] = {
 	"rx_packets", "rx_bytes", "tx_packets", "tx_bytes", "rx_packets_csum",
@@ -618,6 +622,9 @@ static int emac_configure(struct emac_instance *dev)
 	if (emac_phy_gpcs(dev->phy.mode))
 		emac_mii_reset_phy(&dev->phy);
 
+	/* Required for Pause packet support in EMAC */
+	dev_mc_add(ndev, default_mcast_addr, sizeof(default_mcast_addr), 1);
+
 	return 0;
 }
 
-- 
1.5.4.4

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Jeff Garzik]

Stefan Roese wrote:
> From: Pravin M. Bathija <pbathija@amcc.com>
> 
> Problem Description and Fix
> ---------------------------
> When a pause packet(with destination as reserved Multicast address) is
> received by the EMAC hardware to control the flow of frames being
> transmitted by it, it is dropped by the hardware unless the reserved
> Multicast address is hashed in to the GAHT[1-4] registers. This code fix
> adds the default reserved multicast address to the GAHT[1-4] registers
> in the EMAC(s) present on the chip. The flow control with Pause packets
> will only work if the following register bits are programmed in EMAC:
> EMACx_MR1[APP] = 1
> EMACx_RMR[BAE] = 1
> EMACx_RMR[MAE] = 1
> 
> Behavior that may be observed in a running system
> -------------------------------------------------
> A host transferring data from a PPC based system may send a Pause packet
> to the PPC EMAC requesting it to slow down the flow of packets. If the
> default reserved multicast MAC address is not programmed into the
> GAHT[1-4] registers this Pause packet will be dropped by PPC EMAC and no
> Flow Control will be done.
> 
> Signed-off-by: Pravin M. Bathija <pbathija@amcc.com>
> Signed-off-by: Stefan Roese <sr@denx.de>
> ---
>  drivers/net/ibm_newemac/core.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)

applied

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Benjamin Herrenschmidt]

> > 
> > Signed-off-by: Pravin M. Bathija <pbathija@amcc.com>
> > Signed-off-by: Stefan Roese <sr@denx.de>
> > ---
> >  drivers/net/ibm_newemac/core.c |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> applied

Thanks. There's also a couple of patches from Valentine that should go
in. I'll ask him to resend to you.

Cheers,
Ben.

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Benjamin Herrenschmidt]

On Tue, 2008-03-18 at 08:25 +1100, Benjamin Herrenschmidt wrote:
> > > 
> > > Signed-off-by: Pravin M. Bathija <pbathija@amcc.com>
> > > Signed-off-by: Stefan Roese <sr@denx.de>
> > > ---
> > >  drivers/net/ibm_newemac/core.c |    7 +++++++
> > >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > applied
> 
> Thanks. There's also a couple of patches from Valentine that should go
> in. I'll ask him to resend to you.

Actually, they need some arch bits so they are better off going through
Josh tree on top of the said changes.

Valentine, can you make sure Jeff at least has a chance to see them and
ack them though ?

Cheers,
Ben.

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Josh Boyer]

On Tue, 18 Mar 2008 08:26:45 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> 
> On Tue, 2008-03-18 at 08:25 +1100, Benjamin Herrenschmidt wrote:
> > > > 
> > > > Signed-off-by: Pravin M. Bathija <pbathija@amcc.com>
> > > > Signed-off-by: Stefan Roese <sr@denx.de>
> > > > ---
> > > >  drivers/net/ibm_newemac/core.c |    7 +++++++
> > > >  1 files changed, 7 insertions(+), 0 deletions(-)
> > > 
> > > applied
> > 
> > Thanks. There's also a couple of patches from Valentine that should go
> > in. I'll ask him to resend to you.
> 
> Actually, they need some arch bits so they are better off going through
> Josh tree on top of the said changes.

There's also the section mismatch patch I sent you.  I don't care who's
tree they go through, but I'd need to know either way so keep me in the
loop please.

josh

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Benjamin Herrenschmidt]

> There's also the section mismatch patch I sent you.  I don't care who's
> tree they go through, but I'd need to know either way so keep me in the
> loop please.

Jeff, do you mind if we get those patches through the powerpc tree ?
EMAC is very powerpc specific and these are mostly internal driver
cuisine.

Ben.

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Jeff Garzik]

Benjamin Herrenschmidt wrote:
>> There's also the section mismatch patch I sent you.  I don't care who's
>> tree they go through, but I'd need to know either way so keep me in the
>> loop please.
> 
> Jeff, do you mind if we get those patches through the powerpc tree ?
> EMAC is very powerpc specific and these are mostly internal driver
> cuisine.

Fine with me in concept, though I haven't seen the patches in question.

	Jeff

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Benjamin Herrenschmidt]

On Mon, 2008-03-17 at 18:31 -0400, Jeff Garzik wrote:
> > Jeff, do you mind if we get those patches through the powerpc tree ?
> > EMAC is very powerpc specific and these are mostly internal driver
> > cuisine.
> 
> Fine with me in concept, though I haven't seen the patches in
> question.

Yup, I've asked people to copy you on them so you get a chance to ack
them :-)

Cheers,
Ben.

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Josh Boyer]

On Mon, 17 Mar 2008 18:31:53 -0400
Jeff Garzik <jeff@garzik.org> wrote:

> Benjamin Herrenschmidt wrote:
> >> There's also the section mismatch patch I sent you.  I don't care who's
> >> tree they go through, but I'd need to know either way so keep me in the
> >> loop please.
> > 
> > Jeff, do you mind if we get those patches through the powerpc tree ?
> > EMAC is very powerpc specific and these are mostly internal driver
> > cuisine.
> 
> Fine with me in concept, though I haven't seen the patches in question.

http://marc.info/?l=linux-netdev&m=120449748701492&w=2

I sent it to Ben with netdev on CC because you asked the various people
sending NEWEMAC patches to you to find a single person.

So from now on, what are we going to do?  It seems we're playing net
maintainer russian roulette for NEWEMAC and it's getting old.

josh

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Benjamin Herrenschmidt]

> http://marc.info/?l=linux-netdev&m=120449748701492&w=2
> 
> I sent it to Ben with netdev on CC because you asked the various people
> sending NEWEMAC patches to you to find a single person.
> 
> So from now on, what are we going to do?  It seems we're playing net
> maintainer russian roulette for NEWEMAC and it's getting old.

I'm happy to collect the patches. I said I would do it, I was just taken
aback a bit by Jeff suddenly merging two of Stefan patches directly :-)

I was actually planning this week to post a serie of all the pending
EMAC patches for final review.

Cheers,
Ben.

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Josh Boyer]

On Tue, 18 Mar 2008 13:41:40 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> 
> > http://marc.info/?l=linux-netdev&m=120449748701492&w=2
> > 
> > I sent it to Ben with netdev on CC because you asked the various people
> > sending NEWEMAC patches to you to find a single person.
> > 
> > So from now on, what are we going to do?  It seems we're playing net
> > maintainer russian roulette for NEWEMAC and it's getting old.
> 
> I'm happy to collect the patches. I said I would do it, I was just taken
> aback a bit by Jeff suddenly merging two of Stefan patches directly :-)
> 
> I was actually planning this week to post a serie of all the pending
> EMAC patches for final review.

Ok.  Either way, if it helps someone I'd be happy to roll them into a
newemac branch in my git tree.

josh

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Jeff Garzik]

Benjamin Herrenschmidt wrote:
>> http://marc.info/?l=linux-netdev&m=120449748701492&w=2
>>
>> I sent it to Ben with netdev on CC because you asked the various people
>> sending NEWEMAC patches to you to find a single person.
>>
>> So from now on, what are we going to do?  It seems we're playing net
>> maintainer russian roulette for NEWEMAC and it's getting old.
> 
> I'm happy to collect the patches. I said I would do it, I was just taken
> aback a bit by Jeff suddenly merging two of Stefan patches directly :-)

Sorry...  they were CC'd to me and netdev, Stefan had previously 
contributed acceptable patches, and the patches looked OK, so...  :)

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Benjamin Herrenschmidt]

On Tue, 2008-03-18 at 00:06 -0400, Jeff Garzik wrote:
> Benjamin Herrenschmidt wrote:
> >> http://marc.info/?l=linux-netdev&m=120449748701492&w=2
> >>
> >> I sent it to Ben with netdev on CC because you asked the various people
> >> sending NEWEMAC patches to you to find a single person.
> >>
> >> So from now on, what are we going to do?  It seems we're playing net
> >> maintainer russian roulette for NEWEMAC and it's getting old.
> > 
> > I'm happy to collect the patches. I said I would do it, I was just taken
> > aback a bit by Jeff suddenly merging two of Stefan patches directly :-)
> 
> Sorry...  they were CC'd to me and netdev, Stefan had previously 
> contributed acceptable patches, and the patches looked OK, so...  :)

Sure they were fine, it's just a matter of getting the right
procedure :-)

Now, I'll collect the remaining ones and will send them for
review again.

Cheers,
Ben.

Re: [PATCH] net: NEWEMAC: Support for Pause packets in EMAC driver

[Valentine Barshak]

Benjamin Herrenschmidt wrote:
>> http://marc.info/?l=linux-netdev&m=120449748701492&w=2
>>
>> I sent it to Ben with netdev on CC because you asked the various people
>> sending NEWEMAC patches to you to find a single person.
>>
>> So from now on, what are we going to do?  It seems we're playing net
>> maintainer russian roulette for NEWEMAC and it's getting old.
> 
> I'm happy to collect the patches. I said I would do it, I was just taken
> aback a bit by Jeff suddenly merging two of Stefan patches directly :-)
> 
> I was actually planning this week to post a serie of all the pending
> EMAC patches for final review.
> 
> Cheers,
> Ben.
> 
> 

FYI, These are the NEWEMAC patches I've sent:
http://ozlabs.org/pipermail/linuxppc-dev/2008-February/052114.html
(This is a TAH csum fix, that I don't see on the patchwork for some reason)
and a couple of PHY clock workarounds
http://patchwork.ozlabs.org/linuxppc/patch?id=17211
http://patchwork.ozlabs.org/linuxppc/patch?id=17212
The latter two patches depend on the "dcri_clrset" one:
http://patchwork.ozlabs.org/linuxppc/patch?id=17199
Ben, could you please also take a look at the PCIe dcri_clrset stuff here
http://patchwork.ozlabs.org/linuxppc/patch?id=17214
(not EMAC related)
Thanks,
Valentine.