From: linas@austin.ibm.com (Linas Vepstas)
To: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: akpm@osdl.org, ppc-dev <linuxppc-dev@ozlabs.org>,
jgarzik@pobox.com, netdev@vger.kernel.org
Subject: Re: Fw: [PATCH] Remove powerpc specific parts of 3c509 driver
Date: Tue, 19 Sep 2006 13:42:43 -0500 [thread overview]
Message-ID: <20060919184243.GL29167@austin.ibm.com> (raw)
In-Reply-To: <20060919145433.8fc7d478.sfr@canb.auug.org.au>
On Tue, Sep 19, 2006 at 02:54:33PM +1000, Stephen Rothwell wrote:
>
> On powerpc and ppc, insl_ns and insl are identical as are outsl_ns and
> outsl, so remove the conditional use of insl_ns and outsl_ns.
The rest of this patch might indeed be correct, but the above comment
bothers me. The "ns" versions of routines are supposed to be
non-byte-swapped versions of the insl/outsl routines (which would
byte-swap on big-endian archs such as powerpc.)
> diff --git a/drivers/net/3c509.c b/drivers/net/3c509.c
> index cbdae54..add6381 100644
> --- a/drivers/net/3c509.c
> +++ b/drivers/net/3c509.c
> @@ -879,11 +879,7 @@ #endif
> outw(skb->len, ioaddr + TX_FIFO);
> outw(0x00, ioaddr + TX_FIFO);
> /* ... and the packet rounded to a doubleword. */
> -#ifdef __powerpc__
> - outsl_ns(ioaddr + TX_FIFO, skb->data, (skb->len + 3) >> 2);
> -#else
> outsl(ioaddr + TX_FIFO, skb->data, (skb->len + 3) >> 2);
> -#endif
Dohh, a hack like this to work around some possbile byte-swapping
bug should never have been done in the first place :-(
However, I presume someone added the __powerpc__ define here
because they picked up a 3c509 at a garage sale, stuck it in
a powerpc, found out it didn't work due to a byte-swapping bug,
and then patched it as above. I'm disturbed that somehow
outsl_ns() became identical to outsl() at some point, presumably
breaking this patch.
--linas
WARNING: multiple messages have this Message-ID (diff)
From: linas@austin.ibm.com (Linas Vepstas)
To: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: ppc-dev <linuxppc-dev@ozlabs.org>,
netdev@vger.kernel.org, akpm@osdl.org, jgarzik@pobox.com
Subject: Re: Fw: [PATCH] Remove powerpc specific parts of 3c509 driver
Date: Tue, 19 Sep 2006 13:42:43 -0500 [thread overview]
Message-ID: <20060919184243.GL29167@austin.ibm.com> (raw)
In-Reply-To: <20060919145433.8fc7d478.sfr@canb.auug.org.au>
On Tue, Sep 19, 2006 at 02:54:33PM +1000, Stephen Rothwell wrote:
>
> On powerpc and ppc, insl_ns and insl are identical as are outsl_ns and
> outsl, so remove the conditional use of insl_ns and outsl_ns.
The rest of this patch might indeed be correct, but the above comment
bothers me. The "ns" versions of routines are supposed to be
non-byte-swapped versions of the insl/outsl routines (which would
byte-swap on big-endian archs such as powerpc.)
> diff --git a/drivers/net/3c509.c b/drivers/net/3c509.c
> index cbdae54..add6381 100644
> --- a/drivers/net/3c509.c
> +++ b/drivers/net/3c509.c
> @@ -879,11 +879,7 @@ #endif
> outw(skb->len, ioaddr + TX_FIFO);
> outw(0x00, ioaddr + TX_FIFO);
> /* ... and the packet rounded to a doubleword. */
> -#ifdef __powerpc__
> - outsl_ns(ioaddr + TX_FIFO, skb->data, (skb->len + 3) >> 2);
> -#else
> outsl(ioaddr + TX_FIFO, skb->data, (skb->len + 3) >> 2);
> -#endif
Dohh, a hack like this to work around some possbile byte-swapping
bug should never have been done in the first place :-(
However, I presume someone added the __powerpc__ define here
because they picked up a 3c509 at a garage sale, stuck it in
a powerpc, found out it didn't work due to a byte-swapping bug,
and then patched it as above. I'm disturbed that somehow
outsl_ns() became identical to outsl() at some point, presumably
breaking this patch.
--linas
next prev parent reply other threads:[~2006-09-19 18:42 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-09-19 4:54 Fw: [PATCH] Remove powerpc specific parts of 3c509 driver Stephen Rothwell
2006-09-19 18:42 ` Linas Vepstas [this message]
2006-09-19 18:42 ` Linas Vepstas
2006-09-19 18:52 ` Matt Sealey
2006-09-19 18:52 ` Matt Sealey
2006-09-19 19:44 ` Linas Vepstas
2006-09-19 19:44 ` Linas Vepstas
2006-09-19 23:24 ` Benjamin Herrenschmidt
2006-09-19 23:24 ` Benjamin Herrenschmidt
2006-09-20 0:21 ` Segher Boessenkool
2006-09-20 0:25 ` Benjamin Herrenschmidt
2006-09-20 0:58 ` Segher Boessenkool
2006-09-20 0:58 ` Segher Boessenkool
2006-09-20 1:17 ` Linas Vepstas
2006-09-20 1:17 ` Linas Vepstas
2006-09-20 1:41 ` Segher Boessenkool
2006-09-20 0:57 ` Jeff Garzik
2006-09-20 1:08 ` Segher Boessenkool
2006-09-20 1:08 ` Segher Boessenkool
2006-09-20 1:18 ` Jeff Garzik
2006-09-20 1:18 ` Jeff Garzik
2006-09-20 20:06 ` Segher Boessenkool
2006-09-20 20:06 ` Segher Boessenkool
2006-09-20 21:29 ` Jeff Garzik
2006-09-20 21:29 ` Jeff Garzik
2006-09-19 23:18 ` Fw: " Benjamin Herrenschmidt
2006-09-19 23:27 ` Paul Mackerras
2006-09-19 23:27 ` Paul Mackerras
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20060919184243.GL29167@austin.ibm.com \
--to=linas@austin.ibm.com \
--cc=akpm@osdl.org \
--cc=jgarzik@pobox.com \
--cc=linuxppc-dev@ozlabs.org \
--cc=netdev@vger.kernel.org \
--cc=sfr@canb.auug.org.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.