From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wf-out-1314.google.com (wf-out-1314.google.com [209.85.200.174]) by ozlabs.org (Postfix) with ESMTP id AE03DDDF7A for ; Fri, 8 May 2009 12:36:40 +1000 (EST) Received: by wf-out-1314.google.com with SMTP id 24so931512wfg.15 for ; Thu, 07 May 2009 19:36:39 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1241640919-4650-1-git-send-email-wd@denx.de> <1241640919-4650-4-git-send-email-wd@denx.de> <20090506221250.C977183420E8@gemini.denx.de> Date: Thu, 7 May 2009 20:36:39 -0600 Message-ID: <4b73d43f0905071936i9ba8551yc383820dd1ea1831@mail.gmail.com> Subject: Re: [PATCH 03/12] fs_enet: Add FEC TX Alignment workaround for MPC5121. From: John Rigby To: Grant Likely Content-Type: multipart/alternative; boundary=000e0cd229d8839e6604695d7f56 Cc: linuxppc-dev@ozlabs.org, netdev@vger.kernel.org, Wolfgang Denk , Detlev Zundel , Piotr Ziecik List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , --000e0cd229d8839e6604695d7f56 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit I was having deja-vu with this and realized that I have fixed at least some of the objections to this patch. Wolfgang you may want to look at the patch in my 5121 git tree here: http://git.denx.de/?p=linux-mpc512x.git;a=commit;h=2950be3be42af7449941c3340998c27ef918f10f It does runtime tx packet alignment It also has fewer ifdefs and trys to share more code. It also has a header that explains everything including that fact that there is not a runtime conflict sine the only other ppc that has fec is 8xx which is not in the same family. On Wed, May 6, 2009 at 4:42 PM, Grant Likely wrote: > On Wed, May 6, 2009 at 4:12 PM, Wolfgang Denk wrote: > > Dear Grant Likely, > > > > In message > you wrote: > >> > >> > The FEC on 5121 has problems with misaligned tx buffers. > >> > The RM says any alignment is ok but empirical results > >> > show that packet buffers ending in 0x1E will sometimes > >> > hang the FEC. Other bad alignment does not hang but will > >> > cause silent TX failures resulting in about a 1% packet > >> > loss as tested by ping -f from a remote host. > >> > > >> > This patch is a work around that copies every tx packet > >> > to an aligned skb before sending. > >> > >> OUCH! > > > > Yes :-( > > > >> > +#else > >> > +#define tx_skb_align_workaround(dev, skb) (skb) > >> > +#endif > >> > >> Another use of #ifdef blocks. What is the multiplatform impact? > > > > Hm... Can you recommend a better way to solve the problem? Suggestions > > are welcome. > > I'd rather see a runtime selectable workaround. ie. enable it based > on the compatible property. > > g. > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. > --000e0cd229d8839e6604695d7f56 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable I was having deja-vu with this and realized that I have fixed at least some= of the objections to this patch.=A0

Wolfgang you may want to look = at the patch in my 5121 git tree here:

http://git.denx.de/?p=3Dlinux-mpc512x.git;a=3Dcommit;h=3D2950be3be42a= f7449941c3340998c27ef918f10f

It does runtime tx packet alignment=A0 It also has fewer ifdefs and try= s to share more code.=A0 It also has a header that explains everything incl= uding that fact that there is not a runtime conflict sine the only other pp= c that has fec is 8xx which is not in the same family.

On Wed, May 6, 2009 at 4:42 PM, Grant Likely= <grant.l= ikely@secretlab.ca> wrote:
On Wed, May 6, 2009 at 4:12 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Grant Likely,
>
> In message <fa686aa40905061337w6aa82f5aj787618ba108e528f@mail.g= mail.com> you wrote:
>>
>> > The FEC on 5121 has problems with misaligned tx buffers.
>> > The RM says any alignment is ok but empirical results
>> > show that packet buffers ending in 0x1E will sometimes
>> > hang the FEC. Other bad alignment does not hang but will
>> > cause silent TX failures resulting in about a 1% packet
>> > loss as tested by ping -f from a remote host.
>> >
>> > This patch is a work around that copies every tx packet
>> > to an aligned skb before sending.
>>
>> OUCH!
>
> Yes :-(
>
>> > +#else
>> > +#define tx_skb_align_workaround(dev, skb) (skb)
>> > +#endif
>>
>> Another use of #ifdef blocks. =A0What is the multiplatform impact?=
>
> Hm... Can you recommend a better way to solve the problem? Suggestions=
> are welcome.

I'd rather see a runtime selectable workaround. =A0ie. enable it = based
on the compatible property.

g.

--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

--000e0cd229d8839e6604695d7f56--