On Wed, Feb 11, 2015 at 1:08 AM, Stefan Roese sr@denx.de wrote:
(Added Joe Hershberger to Cc, because this discussion is "network"
related and not really SoCFPGA related)
On 10.02.2015 19:51, Marek Vasut wrote:
We already do this kind of a programming in board/altera/socfpga/socfpga.c in board_phy_config(), don't we ?
Yes, good point. This patch series is a first in some upcoming patches
to
make this better. The Linux implementation uses devicetree settings to
set
the skews, so if we were to follow that same model the code in socfpga.c would become deprecated in favor of setting the skews through the phy driver and subsequently removed. That way other users could take
advantage
of this through devicetree.
Setting the skews in DT would indeed be preferable in my opinion.
+1 from me.
+1 from me as well.
The other problem with the current implementation is the skew values are part specific - we set the actual register values in the environment when it would be better to use a
skewed
time value (in +/- picoseconds).
You mean they are specific for particular PHY model ? Or board model ? Or even particular board itself ?
Also, see [1], once I apply this, the DT support (not DM) for SoCFPGA will become mandatory. Won't it make more sense to pull these values from the DT instead of poluting the board environment with those please ?
I agree it would make more sense to pull these from devicetree - I'm planning on adding that in a future patch. I thought it would be a good idea to pull these values from the environment first, overriding the devicetree (if present in the environment). This approach is helpful during bringup & debug since it doesn't require one to change the devicetree to try something quickly. I'm ok with any approach you think would work for the community.
You can do that with the 'mii' command as well I think, but I might be
wrong.
Yes. For testing or board bringup this might really serve. Even though
this setting via environment as proposed from Vince is more elegant and less hackish. And easier to adjust/tune for "normal users".
The default values should come from the DT, once this is all in place.
But I think that for initial board bringup / testing such a method, to override those values via environment variables can be quite helpful.
Joe, whats your opinion on this?
Do we really think this is a strong use-case? This seem like the type of thing I would expect to tweak for testing through mii / mdio commands and then configure the device tree based on that. This is pretty much a one-time thing for a given board it seems to me.
If we really want a polished interface to it, we should define a sub-command / new command that phy drivers can implement. I'm not sure an undiscoverable, un-"help"-able list of env vars will be apparent to users. Do you have a feeling for how close to universal any of these parameters are across phys?
-Joe