Re: [U-Boot] [PATCH] [resend] net: fman: fix 2.5G SGMII settings

[Joe Hershberger]

On Mon, Nov 14, 2016 at 1:27 AM, Shaohui Xie  wrote:
> The settings for 2.5G SGMII are wrong, which the 2.5G case is missed in
> set_if_mode(), and the serdes PCS configuration are wrong, this patch uses
> the correct settings took from Linux.
>
> Signed-off-by: Shaohui Xie 
> ---
> not sure what was wrong, the patch did not show in patchwork, so resend it.
> Sorry for the bothering.
>
>  drivers/net/fm/eth.c   | 32 ++--
>  drivers/net/fm/memac.c |  1 +
>  include/fsl_memac.h|  1 +
>  3 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c
> index eb8e936..543aaa8 100644
> --- a/drivers/net/fm/eth.c
> +++ b/drivers/net/fm/eth.c
> @@ -45,9 +45,11 @@ static void dtsec_configure_serdes(struct fm_eth *priv)
>
>  qsgmii_loop:
> /* SGMII IF mode + AN enable only for 1G SGMII, not for 2.5G */
> -   value = PHY_SGMII_IF_MODE_SGMII;
> -   if (!sgmii_2500)
> -   value |= PHY_SGMII_IF_MODE_AN;
> +   value = PHY_SGMII_IF_MODE_SGMII | PHY_SGMII_IF_MODE_AN;
> +   if (sgmii_2500)
> +   value = PHY_SGMII_CR_PHY_RESET |
> +   PHY_SGMII_IF_SPEED_GIGABIT |
> +   PHY_SGMII_IF_MODE_SGMII;

This is an odd style for readability. Since you are overwriting value
in the case of sgmii_2500, why not use something like:

> +   if (sgmii_2500)
> +   value = PHY_SGMII_CR_PHY_RESET |
> +   PHY_SGMII_IF_SPEED_GIGABIT |
> +   PHY_SGMII_IF_MODE_SGMII;
> +   else
> +   value = PHY_SGMII_IF_MODE_SGMII | PHY_SGMII_IF_MODE_AN;

(reformatted of course)

> memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x14, value);
>
> @@ -55,15 +57,24 @@ qsgmii_loop:
> value = PHY_SGMII_DEV_ABILITY_SGMII;
> memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x4, value);
>
> -   /* Adjust link timer for SGMII  -
> -   1.6 ms in units of 8 ns = 2 * 10^5 = 0x30d40 */
> -   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x13, 0x3);
> -   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x12, 0xd40);
> +   if (sgmii_2500) {
> +   /* Adjust link timer for 2.5G SGMII,
> +* 1.6 ms in units of 3.2 ns:
> +* 1.6ms / 3.2ns = 5 * 10^5 = 0x7a120.
> +*/
> +   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x13, 0x0007);
> +   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x12, 0xa120);
> +   } else {
> +   /* Adjust link timer for SGMII,
> +* 1.6 ms in units of 8 ns:
> +* 1.6ms / 8ns = 2 * 10^5 = 0x30d40.
> +*/
> +   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x13, 0x3);
> +   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x12, 0xd40);

Please add the leading 0's like you did in the other case to make it clearer.

> +   }
>
> /* Restart AN */
> -   value = PHY_SGMII_CR_DEF_VAL;
> -   if (!sgmii_2500)
> -   value |= PHY_SGMII_CR_RESET_AN;
> +   value = PHY_SGMII_CR_DEF_VAL | PHY_SGMII_CR_RESET_AN;
> memac_mdio_write(, i, MDIO_DEVAD_NONE, 0, value);
>
> if ((priv->enet_if == PHY_INTERFACE_MODE_QSGMII) && (i < 3)) {
> @@ -391,6 +402,7 @@ static int fm_eth_startup(struct fm_eth *fm_eth)
>
> /* For some reason we need to set SPEED_100 */
> if (((fm_eth->enet_if == PHY_INTERFACE_MODE_SGMII) ||
> +(fm_eth->enet_if == PHY_INTERFACE_MODE_SGMII_2500) ||
>  (fm_eth->enet_if == PHY_INTERFACE_MODE_QSGMII)) &&
>   mac->set_if_mode)
> mac->set_if_mode(mac, fm_eth->enet_if, SPEED_100);
> diff --git a/drivers/net/fm/memac.c b/drivers/net/fm/memac.c
> index 81a64bf..1b5779c 100644
> --- a/drivers/net/fm/memac.c
> +++ b/drivers/net/fm/memac.c
> @@ -90,6 +90,7 @@ static void memac_set_interface_mode(struct fsl_enet_mac 
> *mac,
> if_mode |= (IF_MODE_GMII | IF_MODE_RM);
> break;
> case PHY_INTERFACE_MODE_SGMII:
> +   case PHY_INTERFACE_MODE_SGMII_2500:
> case PHY_INTERFACE_MODE_QSGMII:
> if_mode &= ~IF_MODE_MASK;
> if_mode |= (IF_MODE_GMII);
> diff --git a/include/fsl_memac.h b/include/fsl_memac.h
> index bed2a40..431c2a0 100644
> --- a/include/fsl_memac.h
> +++ b/include/fsl_memac.h
> @@ -226,6 +226,7 @@ struct memac {
>  #define PHY_SGMII_CR_PHY_RESET  0x8000
>  #define PHY_SGMII_CR_RESET_AN   0x0200
>  #define PHY_SGMII_CR_DEF_VAL0x1140
> +#define PHY_SGMII_IF_SPEED_GIGABIT  0x0008
>  #define PHY_SGMII_DEV_ABILITY_SGMII 0x4001
>  #define PHY_SGMII_IF_MODE_AN0x0002
>  #define PHY_SGMII_IF_MODE_SGMII 0x0001
> --
> 2.1.0.27.g96db324
>
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
___
U-Boot 

[U-Boot] [PATCH] [resend] net: fman: fix 2.5G SGMII settings

[shh.xie]

From: Shaohui Xie 

The settings for 2.5G SGMII are wrong, which the 2.5G case is missed in
set_if_mode(), and the serdes PCS configuration are wrong, this patch uses
the correct settings took from Linux.

Signed-off-by: Shaohui Xie 
---
not sure what was wrong, the patch did not show in patchwork, so resend it.
Sorry for the bothering.

 drivers/net/fm/eth.c   | 32 ++--
 drivers/net/fm/memac.c |  1 +
 include/fsl_memac.h|  1 +
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c
index eb8e936..543aaa8 100644
--- a/drivers/net/fm/eth.c
+++ b/drivers/net/fm/eth.c
@@ -45,9 +45,11 @@ static void dtsec_configure_serdes(struct fm_eth *priv)
 
 qsgmii_loop:
/* SGMII IF mode + AN enable only for 1G SGMII, not for 2.5G */
-   value = PHY_SGMII_IF_MODE_SGMII;
-   if (!sgmii_2500)
-   value |= PHY_SGMII_IF_MODE_AN;
+   value = PHY_SGMII_IF_MODE_SGMII | PHY_SGMII_IF_MODE_AN;
+   if (sgmii_2500)
+   value = PHY_SGMII_CR_PHY_RESET |
+   PHY_SGMII_IF_SPEED_GIGABIT |
+   PHY_SGMII_IF_MODE_SGMII;
 
memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x14, value);
 
@@ -55,15 +57,24 @@ qsgmii_loop:
value = PHY_SGMII_DEV_ABILITY_SGMII;
memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x4, value);
 
-   /* Adjust link timer for SGMII  -
-   1.6 ms in units of 8 ns = 2 * 10^5 = 0x30d40 */
-   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x13, 0x3);
-   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x12, 0xd40);
+   if (sgmii_2500) {
+   /* Adjust link timer for 2.5G SGMII,
+* 1.6 ms in units of 3.2 ns:
+* 1.6ms / 3.2ns = 5 * 10^5 = 0x7a120.
+*/
+   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x13, 0x0007);
+   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x12, 0xa120);
+   } else {
+   /* Adjust link timer for SGMII,
+* 1.6 ms in units of 8 ns:
+* 1.6ms / 8ns = 2 * 10^5 = 0x30d40.
+*/
+   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x13, 0x3);
+   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x12, 0xd40);
+   }
 
/* Restart AN */
-   value = PHY_SGMII_CR_DEF_VAL;
-   if (!sgmii_2500)
-   value |= PHY_SGMII_CR_RESET_AN;
+   value = PHY_SGMII_CR_DEF_VAL | PHY_SGMII_CR_RESET_AN;
memac_mdio_write(, i, MDIO_DEVAD_NONE, 0, value);
 
if ((priv->enet_if == PHY_INTERFACE_MODE_QSGMII) && (i < 3)) {
@@ -391,6 +402,7 @@ static int fm_eth_startup(struct fm_eth *fm_eth)
 
/* For some reason we need to set SPEED_100 */
if (((fm_eth->enet_if == PHY_INTERFACE_MODE_SGMII) ||
+(fm_eth->enet_if == PHY_INTERFACE_MODE_SGMII_2500) ||
 (fm_eth->enet_if == PHY_INTERFACE_MODE_QSGMII)) &&
  mac->set_if_mode)
mac->set_if_mode(mac, fm_eth->enet_if, SPEED_100);
diff --git a/drivers/net/fm/memac.c b/drivers/net/fm/memac.c
index 81a64bf..1b5779c 100644
--- a/drivers/net/fm/memac.c
+++ b/drivers/net/fm/memac.c
@@ -90,6 +90,7 @@ static void memac_set_interface_mode(struct fsl_enet_mac *mac,
if_mode |= (IF_MODE_GMII | IF_MODE_RM);
break;
case PHY_INTERFACE_MODE_SGMII:
+   case PHY_INTERFACE_MODE_SGMII_2500:
case PHY_INTERFACE_MODE_QSGMII:
if_mode &= ~IF_MODE_MASK;
if_mode |= (IF_MODE_GMII);
diff --git a/include/fsl_memac.h b/include/fsl_memac.h
index bed2a40..431c2a0 100644
--- a/include/fsl_memac.h
+++ b/include/fsl_memac.h
@@ -226,6 +226,7 @@ struct memac {
 #define PHY_SGMII_CR_PHY_RESET  0x8000
 #define PHY_SGMII_CR_RESET_AN   0x0200
 #define PHY_SGMII_CR_DEF_VAL0x1140
+#define PHY_SGMII_IF_SPEED_GIGABIT  0x0008
 #define PHY_SGMII_DEV_ABILITY_SGMII 0x4001
 #define PHY_SGMII_IF_MODE_AN0x0002
 #define PHY_SGMII_IF_MODE_SGMII 0x0001
-- 
2.1.0.27.g96db324

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] [resend] net: fman: fix 2.5G SGMII settings

[Shaohui Xie]

The settings for 2.5G SGMII are wrong, which the 2.5G case is missed in
set_if_mode(), and the serdes PCS configuration are wrong, this patch uses
the correct settings took from Linux.

Signed-off-by: Shaohui Xie 
---
not sure what was wrong, the patch did not show in patchwork, so resend it.
Sorry for the bothering.

 drivers/net/fm/eth.c   | 32 ++--
 drivers/net/fm/memac.c |  1 +
 include/fsl_memac.h|  1 +
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c
index eb8e936..543aaa8 100644
--- a/drivers/net/fm/eth.c
+++ b/drivers/net/fm/eth.c
@@ -45,9 +45,11 @@ static void dtsec_configure_serdes(struct fm_eth *priv)
 
 qsgmii_loop:
/* SGMII IF mode + AN enable only for 1G SGMII, not for 2.5G */
-   value = PHY_SGMII_IF_MODE_SGMII;
-   if (!sgmii_2500)
-   value |= PHY_SGMII_IF_MODE_AN;
+   value = PHY_SGMII_IF_MODE_SGMII | PHY_SGMII_IF_MODE_AN;
+   if (sgmii_2500)
+   value = PHY_SGMII_CR_PHY_RESET |
+   PHY_SGMII_IF_SPEED_GIGABIT |
+   PHY_SGMII_IF_MODE_SGMII;
 
memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x14, value);
 
@@ -55,15 +57,24 @@ qsgmii_loop:
value = PHY_SGMII_DEV_ABILITY_SGMII;
memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x4, value);
 
-   /* Adjust link timer for SGMII  -
-   1.6 ms in units of 8 ns = 2 * 10^5 = 0x30d40 */
-   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x13, 0x3);
-   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x12, 0xd40);
+   if (sgmii_2500) {
+   /* Adjust link timer for 2.5G SGMII,
+* 1.6 ms in units of 3.2 ns:
+* 1.6ms / 3.2ns = 5 * 10^5 = 0x7a120.
+*/
+   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x13, 0x0007);
+   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x12, 0xa120);
+   } else {
+   /* Adjust link timer for SGMII,
+* 1.6 ms in units of 8 ns:
+* 1.6ms / 8ns = 2 * 10^5 = 0x30d40.
+*/
+   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x13, 0x3);
+   memac_mdio_write(, i, MDIO_DEVAD_NONE, 0x12, 0xd40);
+   }
 
/* Restart AN */
-   value = PHY_SGMII_CR_DEF_VAL;
-   if (!sgmii_2500)
-   value |= PHY_SGMII_CR_RESET_AN;
+   value = PHY_SGMII_CR_DEF_VAL | PHY_SGMII_CR_RESET_AN;
memac_mdio_write(, i, MDIO_DEVAD_NONE, 0, value);
 
if ((priv->enet_if == PHY_INTERFACE_MODE_QSGMII) && (i < 3)) {
@@ -391,6 +402,7 @@ static int fm_eth_startup(struct fm_eth *fm_eth)
 
/* For some reason we need to set SPEED_100 */
if (((fm_eth->enet_if == PHY_INTERFACE_MODE_SGMII) ||
+(fm_eth->enet_if == PHY_INTERFACE_MODE_SGMII_2500) ||
 (fm_eth->enet_if == PHY_INTERFACE_MODE_QSGMII)) &&
  mac->set_if_mode)
mac->set_if_mode(mac, fm_eth->enet_if, SPEED_100);
diff --git a/drivers/net/fm/memac.c b/drivers/net/fm/memac.c
index 81a64bf..1b5779c 100644
--- a/drivers/net/fm/memac.c
+++ b/drivers/net/fm/memac.c
@@ -90,6 +90,7 @@ static void memac_set_interface_mode(struct fsl_enet_mac *mac,
if_mode |= (IF_MODE_GMII | IF_MODE_RM);
break;
case PHY_INTERFACE_MODE_SGMII:
+   case PHY_INTERFACE_MODE_SGMII_2500:
case PHY_INTERFACE_MODE_QSGMII:
if_mode &= ~IF_MODE_MASK;
if_mode |= (IF_MODE_GMII);
diff --git a/include/fsl_memac.h b/include/fsl_memac.h
index bed2a40..431c2a0 100644
--- a/include/fsl_memac.h
+++ b/include/fsl_memac.h
@@ -226,6 +226,7 @@ struct memac {
 #define PHY_SGMII_CR_PHY_RESET  0x8000
 #define PHY_SGMII_CR_RESET_AN   0x0200
 #define PHY_SGMII_CR_DEF_VAL0x1140
+#define PHY_SGMII_IF_SPEED_GIGABIT  0x0008
 #define PHY_SGMII_DEV_ABILITY_SGMII 0x4001
 #define PHY_SGMII_IF_MODE_AN0x0002
 #define PHY_SGMII_IF_MODE_SGMII 0x0001
-- 
2.1.0.27.g96db324

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot