[U-Boot] [PATCH] net: phy: micrel: fix KSZ9031 clock skew for values greater 0ps

[U-Boot] [PATCH] net: phy: micrel: fix KSZ9031 clock skew for values greater 0ps

[Andreas Pretzsch]

For KSZ9021, all skew register fields are 4-bit wide.
For KSZ9031, the clock skew register fields are 5-bit wide.

The common code in ksz90x1_of_config_group calculating the combined
register value checks if the requested value is above the maximum
and uses this maximum if so. The calculation of this maximum uses
the register width, but the check itself does not. It uses a hardcoded
value of 0xf, which is too low in case of the 5-bit clock (0x1f).
This detail was probably lost during driver unification.

Effect (only for KSZ9031 clock skews): For values greater 900 (== 0ps),
this silently results in 1860 (== +960ps) instead of the requested one.

Fix the check by using the bit width instead of hardcoded value(s).

Signed-off-by: Andreas Pretzsch <apr@cn-eng.de>
---
 drivers/net/phy/micrel_ksz90x1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/micrel_ksz90x1.c b/drivers/net/phy/micrel_ksz90x1.c
index 3951535bf1..63e7b0242b 100644
--- a/drivers/net/phy/micrel_ksz90x1.c
+++ b/drivers/net/phy/micrel_ksz90x1.c
@@ -123,8 +123,8 @@ static int ksz90x1_of_config_group(struct phy_device *phydev,
 		} else {
 			changed = 1;	/* Value was changed in OF */
 			/* Calculate the register value and fix corner cases */
-			if (val[i] > ps_to_regval * 0xf) {
-				max = (1 << ofcfg->grp[i].size) - 1;
+			max = (1 << ofcfg->grp[i].size) - 1;
+			if (val[i] > ps_to_regval * max) {
 				regval |= max << offset;
 			} else {
 				regval |= (val[i] / ps_to_regval) << offset;
-- 
2.19.1

[U-Boot] [PATCH] net: phy: micrel: fix KSZ9031 clock skew for values greater 0ps

[Andreas Pretzsch]

Request for inclusion of below patch.
CC Joe Hershberger as listed maintainer for 'drivers/net/'.

Thanks,
  Andreas

On Thu, 2018-11-29 at 20:04 +0100, Andreas Pretzsch wrote:
> For KSZ9021, all skew register fields are 4-bit wide.
> For KSZ9031, the clock skew register fields are 5-bit wide.
> 
> The common code in ksz90x1_of_config_group calculating the combined
> register value checks if the requested value is above the maximum
> and uses this maximum if so. The calculation of this maximum uses
> the register width, but the check itself does not. It uses a
> hardcoded
> value of 0xf, which is too low in case of the 5-bit clock (0x1f).
> This detail was probably lost during driver unification.
> 
> Effect (only for KSZ9031 clock skews): For values greater 900 (==
> 0ps),
> this silently results in 1860 (== +960ps) instead of the requested
> one.
> 
> Fix the check by using the bit width instead of hardcoded value(s).
> 
> Signed-off-by: Andreas Pretzsch <apr@cn-eng.de>
> ---
>  drivers/net/phy/micrel_ksz90x1.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel_ksz90x1.c
> b/drivers/net/phy/micrel_ksz90x1.c
> index 3951535bf1..63e7b0242b 100644
> --- a/drivers/net/phy/micrel_ksz90x1.c
> +++ b/drivers/net/phy/micrel_ksz90x1.c
> @@ -123,8 +123,8 @@ static int ksz90x1_of_config_group(struct
> phy_device *phydev,
>  		} else {
>  			changed = 1;	/* Value was changed in OF */
>  			/* Calculate the register value and fix corner
> cases */
> -			if (val[i] > ps_to_regval * 0xf) {
> -				max = (1 << ofcfg->grp[i].size) - 1;
> +			max = (1 << ofcfg->grp[i].size) - 1;
> +			if (val[i] > ps_to_regval * max) {
>  				regval |= max << offset;
>  			} else {
>  				regval |= (val[i] / ps_to_regval) <<
> offset;

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch        Tel. +49-(0)7307-936088-1
Lange Strasse 28a                       Fax: +49-(0)7307-936088-9
89250 Senden, Germany                   email: apr at cn-eng.de

[U-Boot] [PATCH] net: phy: micrel: fix KSZ9031 clock skew for values greater 0ps

[Joe Hershberger]

On Thu, Nov 29, 2018 at 1:05 PM Andreas Pretzsch <apr@cn-eng.de> wrote:
>
> For KSZ9021, all skew register fields are 4-bit wide.
> For KSZ9031, the clock skew register fields are 5-bit wide.
>
> The common code in ksz90x1_of_config_group calculating the combined
> register value checks if the requested value is above the maximum
> and uses this maximum if so. The calculation of this maximum uses
> the register width, but the check itself does not. It uses a hardcoded
> value of 0xf, which is too low in case of the 5-bit clock (0x1f).
> This detail was probably lost during driver unification.
>
> Effect (only for KSZ9031 clock skews): For values greater 900 (== 0ps),
> this silently results in 1860 (== +960ps) instead of the requested one.
>
> Fix the check by using the bit width instead of hardcoded value(s).
>
> Signed-off-by: Andreas Pretzsch <apr@cn-eng.de>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] net: phy: micrel: fix KSZ9031 clock skew for values greater 0ps

[Joe Hershberger]

Hi Andreas,

https://patchwork.ozlabs.org/patch/1005603/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe