Subject: Re: [PATCH] gianfar: Revive the driver for eTSEC devices (disable timestamping)

From: Anton Vorontsov <[email-masked]>
Date: Thu, 10 Jun 2010 13:31:59 +0400

On Thu, Jun 10, 2010 at 08:29:08AM +0200, Richard Cochran wrote:
> On Wed, Jun 09, 2010 at 11:32:19PM +0400, Anton Vorontsov wrote:
> > Since commit cc772ab7cdcaa24d1fae332d92a1602788644f7a ("gianfar: Add
> > hardware RX timestamping support"), the driver no longer works on
> > at least MPC8313ERDB and MPC8568EMDS boards (and possibly much more
> > boards as well).
> 
> What do you mean by, "no longer works?" The driver works fine for us,

For me it doesn't even able to receive DHCP answer, boot logs
attached. tcpdump on the host side:

12:56:15.266562 IP 0.0.0.0.68 > 255.255.255.255.67: BOOTP/DHCP, Request from 00:e0:0c:00:7e:21, length 548
12:56:15.266697 IP 10.0.0.2.67 > 10.0.0.32.68: BOOTP/DHCP, Reply, length 324
12:56:22.106691 IP 0.0.0.0.68 > 255.255.255.255.67: BOOTP/DHCP, Request from 00:e0:0c:00:7e:21, length 548
12:56:22.106843 IP 10.0.0.2.67 > 10.0.0.32.68: BOOTP/DHCP, Reply, length 324
12:56:33.698908 IP 0.0.0.0.68 > 255.255.255.255.67: BOOTP/DHCP, Request from 00:e0:0c:00:7e:21, length 548
12:56:33.699083 IP 10.0.0.2.67 > 10.0.0.32.68: BOOTP/DHCP, Reply, length 324
12:56:53.607275 IP 0.0.0.0.68 > 255.255.255.255.67: BOOTP/DHCP, Request from 00:e0:0c:00:7e:21, length 548
12:56:53.607430 IP 10.0.0.2.67 > 10.0.0.32.68: BOOTP/DHCP, Reply, length 324

> > That's how MPC8313 Reference Manual describes RCTRL_TS_ENABLE bit:
> > 
> >   Timestamp incoming packets as padding bytes. PAL field is set
> >   to 8 if the PAL field is programmed to less than 8. Must be set
> >   to zero if TMR_CTRL[TE]=0.
> > 
> > I see that the commit above sets this bit, but it doesn't handle
> > TMR_CTRL. Manfred probably had this bit set by the firmware for
> > his boards. But obviously this isn't true for all boards in the
> > wild.
> 
> No, we did not set TMR_CTRL[TE].

So, you deliberately violate the spec and expect it to work
everywhere? :-)

> For the Rx timestamps, we simply enabled them unconditionally. The
> effect of not setting TMR_CTRL[TE] was that the timestamps were
> invalid, but that should not matter if user space has not configured
> the PTP clock.

It seems to matter here, but I haven't tested if enabling the
TE bit actually solves the problem. Disabling timestamping
makes the driver work for sure though.

> > Also, I recall that Freescale BSPs were explicitly disabling the
> > timestamping because of a performance drop.
> 
> The BSPs that we have, for the MPC8313ERDB and the P2020RBD both
> include a (hacky) PTP timestmaping driver. Can you be more specific
> about where and when Freescale is disabling timestamping?

Well, bitshrine site no longer allows[1] to list patches at
http://www.bitshrine.org/gpp/ , but there was a patch for sure.

I'll try to find it in a bunch of .iso files that I have on disk
tho.

> > For now, the best way to deal with this is just disable the
> > timestamping, and later we can discuss proper device tree bindings
> > and implement enabling this feature via some property.
> 
> Okay, but now we want to identify what exactly works and what not. As
> mentioned, we tested this driver on four different boards and did not
> see any problems.

Sure thing, I would happily test any patches. Or if you need
to dump some register from my board, feel free to send a patch
that adds a bunch of printks and I'll send the output to you.

Thanks,

[1] http://lists.nongnu.org/archive/html/ltib/2010-04/msg00192.html

-- 
Anton Vorontsov
email: [email-masked]
irc://irc.freenode.net/bd2