[patch] net: micrel: configure skew values for

[patch] net: micrel: configure skew values for

[Pavel Machek]

it is cleaner then <20141113120956.GB30779@amd> version. I don't think
I received any reply there.

But the patch you have just merged would interfere with my approach,
and makes it unneccessary, so I'll let you solve it.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [patch] net: micrel: configure skew values for

[Marek Vasut]

On Saturday, October 25, 2014 at 01:27:52 PM, Pavel Machek wrote:
> This adds skew timing configuration for micrel ksz9021
> configuration. With this patch, I can add
> 
> #define CONFIG_KSZ9021_CLK_SKEW_ENV    "micrel-ksz9021-clk-skew"
> #define CONFIG_KSZ9021_CLK_SKEW_VAL    0xf0f0
> #define CONFIG_KSZ9021_DATA_SKEW_ENV   "micrel-ksz9021-data-skew"
> #define CONFIG_KSZ9021_DATA_SKEW_VAL   0x0
> 
> to my board's configuration file, and get working networking in
> u-boot.
> 
> Signed-off-by: Pavel Machek <pavel@denx.de>

CC boundary devices guys, since they already have this on Nitrogen6x,
but they placed the PHY tuning into the board file (which I think is
more correct).

See the attachment, I think this might be a stab on what you want to do.

Also, I don't think it's necessary to explicitly define new ad-hoc env
variables, actually I doubt there's any need for any env variables at all.

Best regards,
Marek Vasut
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-arm-socfpga-board2-Add-Micrel-PHY-tuning.patch
Type: text/x-patch
Size: 1919 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141025/973d4091/attachment.bin>

[U-Boot] [patch] net: micrel: configure skew values for

[Marek Vasut]

On Saturday, October 25, 2014 at 01:27:52 PM, Pavel Machek wrote:
> This adds skew timing configuration for micrel ksz9021
> configuration. With this patch, I can add
> 
> #define CONFIG_KSZ9021_CLK_SKEW_ENV    "micrel-ksz9021-clk-skew"
> #define CONFIG_KSZ9021_CLK_SKEW_VAL    0xf0f0
> #define CONFIG_KSZ9021_DATA_SKEW_ENV   "micrel-ksz9021-data-skew"
> #define CONFIG_KSZ9021_DATA_SKEW_VAL   0x0
> 
> to my board's configuration file, and get working networking in
> u-boot.
> 
> Signed-off-by: Pavel Machek <pavel@denx.de>
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 507b9a3..7e4dbd6 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -117,12 +119,31 @@ static int ksz9021_config(struct phy_device *phydev)
>  {
>  	unsigned ctrl1000 = 0;
>  	const unsigned master = CTRL1000_PREFER_MASTER |
> -			CTRL1000_CONFIG_MASTER | CTRL1000_MANUAL_CONFIG;
> +				CTRL1000_CONFIG_MASTER | CTRL1000_MANUAL_CONFIG;
>  	unsigned features = phydev->drv->features;
> 
> +	/* min rx data delay */
> +	if (ksz9021_phy_extended_write(phydev,
> +				       MII_KSZ9021_EXT_RGMII_RX_DATA_SKEW,
> +				       
getenv_ulong(CONFIG_KSZ9021_DATA_SKEW_ENV, 16,
> +						    
CONFIG_KSZ9021_DATA_SKEW_VAL)) < 0)
> +		return -1;

You should probably get rid of the getenv altogether and just let the board
define a static value in it's configuration (for now). In the long run, this
configuration value should be passed via the platform data or DT.

The new CONFIG_* macros should be documented, otherwise the sheer amount of
CONFIG_* in U-Boot will become impossible to manage. Speaking of CONFIG_*,
given that there is no default value for any of the new CONFIG_* macros, the
patch will break all boards using the Micrel PHY which do not define those
macros explicitly.

On a final note, not all boards using the Micrel PHY need to override those
registers, so this must also be solved in some way such that the behavior
of this driver doesn't change for the boards which do not implement this
override.

I am also curious if there will be more registers which would need such explicit 
override configuration option in the future or if these are all of them. I guess
we cannot tell right now.

[...]

Best regards,
Marek Vasut