[U-Boot] [PATCH 2/2] net: phy: Add ability to program the ksz9031 skew values from the uboot env

[Vince Bridgers]

Hi Marek! Comments and answers follow ....

> Hi Vince!
> > > 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.

Understood. The ability to set skew values through environment variables only helps the debug/bringup phase, so will focus on devicetree settings compatible with the current methods defined in Linux for the Micrel ksz9021 and ksz9031 phy. Per your comment below, we can always edit phy registers from the uboot environment through the mii command. 

> > 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 ?

The current settings are a "value" where each count of the value represents a particular number of picoseconds. The ksz9021 and ksz9031 use different picoseconds per count, so the values currently set are specific to each part. This came out when we brought up the ksz9031 on a new board compared to the ksz9021 on the Cyclone 5 board. A better way to implement this would be to follow this Linux model where skew settings are specified in picoseconds and the phy driver converts picoseconds to a value to be written to a register - and comprehends the specific part's capabilities. 

> > > 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, this is true. I'm ok with dropping the idea of environment variable overrides and focusing on a devicetree based implementation for setting the PHY skew values as needed for particular components or boards.