public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms PATCH v1 0/3] Robust Netsec Initialiation
@ 2019-07-22 11:56 Masahisa Kojima
  2019-07-22 11:56 ` [edk2-platforms PATCH v1 1/3] NetsecDxe: embed phy address into NETSEC SDK internal structure Masahisa Kojima
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Masahisa Kojima @ 2019-07-22 11:56 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif.lindholm, okamoto.satoru, Masahisa Kojima

This patch series is bugfix for the hang-up issue in Netsec driver.

Some linux distributions such as Ubuntu power down the ethernet phy
in reboot. In this case, Netsec initialization fails and
system hungs.

This patch series add the robust netsec initialization,
set ethernet phy as loopback mode to expect stable RXCLK,
and wait for media link up.

The disadvantage of this patch series is that user has to wait
several seconds until netsec driver gives up ethernet link-up
if the ethernet cable is not connected.

Masahisa Kojima (3):
  NetsecDxe: embed phy address into NETSEC SDK internal structure
  NetsecDxe: put phy in loopback mode in order to guarantee stable RXCLK
    input
  NetsecDxe: SnpInitialize() waits for media linking up

 .../Socionext/DeveloperBox/DeveloperBox.dsc   |   1 +
 .../Drivers/Net/NetsecDxe/NetsecDxe.c         | 236 ++++++++----------
 .../Drivers/Net/NetsecDxe/NetsecDxe.dec       |   1 +
 .../Drivers/Net/NetsecDxe/NetsecDxe.h         |   2 -
 .../Drivers/Net/NetsecDxe/NetsecDxe.inf       |   1 +
 .../netsec_sdk/include/ogma_api.h             |   6 +-
 .../netsec_sdk/src/ogma_gmac_access.c         |  61 ++---
 .../netsec_sdk/src/ogma_internal.h            |   2 +
 .../netsec_sdk/src/ogma_misc.c                |  78 +++++-
 .../netsec_for_uefi/netsec_sdk/src/ogma_reg.h |   4 +
 10 files changed, 210 insertions(+), 182 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [edk2-platforms PATCH v1 1/3] NetsecDxe: embed phy address into NETSEC SDK internal structure
  2019-07-22 11:56 [edk2-platforms PATCH v1 0/3] Robust Netsec Initialiation Masahisa Kojima
@ 2019-07-22 11:56 ` Masahisa Kojima
  2019-07-23 15:41   ` Leif Lindholm
  2019-07-22 11:56 ` [edk2-platforms PATCH v1 2/3] NetsecDxe: put phy in loopback mode in order to guarantee stable RXCLK input Masahisa Kojima
  2019-07-22 11:56 ` [edk2-platforms PATCH v1 3/3] NetsecDxe: SnpInitialize() waits for media linking up Masahisa Kojima
  2 siblings, 1 reply; 8+ messages in thread
From: Masahisa Kojima @ 2019-07-22 11:56 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif.lindholm, okamoto.satoru, Masahisa Kojima

This is a refactoring of phy address handling in Netsec driver.
NETSEC SDK, low level driver for NetsecDxe, did not store phy address.
User should specify the phy address as an argument to
the SDK public functions.
It prevented NETSEC SDK from internally controlling phy,
and it also bothers user application with phy address management.

With that, we encapsulate the phy address into NETSEC SDK.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Signed-off-by: Satoru Okamoto <okamoto.satoru@socionext.com>
---
 .../Drivers/Net/NetsecDxe/NetsecDxe.c         | 10 +--
 .../Drivers/Net/NetsecDxe/NetsecDxe.h         |  2 -
 .../netsec_sdk/include/ogma_api.h             |  6 +-
 .../netsec_sdk/src/ogma_gmac_access.c         | 61 +++++--------------
 .../netsec_sdk/src/ogma_internal.h            |  2 +
 .../netsec_sdk/src/ogma_misc.c                |  6 ++
 6 files changed, 28 insertions(+), 59 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
index 160bb08a4632..0b91d4af44a3 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
@@ -59,6 +59,8 @@ Probe (
   // phy-interface
   Param.gmac_config.phy_interface = OGMA_PHY_INTERFACE_RGMII;
 
+  Param.phy_addr = LanDriver->Dev->Resources[2].AddrRangeMin;
+
   // Read and save the Permanent MAC Address
   EepromBase = LanDriver->Dev->Resources[1].AddrRangeMin;
   GetCurrentMacAddress (EepromBase, LanDriver->SnpMode.PermanentAddress.Addr);
@@ -107,8 +109,6 @@ Probe (
     return EFI_DEVICE_ERROR;
   }
 
-  LanDriver->PhyAddress = LanDriver->Dev->Resources[2].AddrRangeMin;
-
   ogma_enable_top_irq (LanDriver->Handle,
                        OGMA_TOP_IRQ_REG_NRM_RX | OGMA_TOP_IRQ_REG_NRM_TX);
 
@@ -280,7 +280,7 @@ SnpInitialize (
     ReturnUnlock (EFI_DEVICE_ERROR);
   }
 
-  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, LanDriver->PhyAddress,
+  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
                &phy_link_status);
   if (ogma_err != OGMA_ERR_OK) {
     DEBUG ((DEBUG_ERROR,
@@ -438,7 +438,7 @@ NetsecPollPhyStatus (
   LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
 
   // Update the media status
-  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, LanDriver->PhyAddress,
+  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
                &phy_link_status);
   if (ogma_err != OGMA_ERR_OK) {
     DEBUG ((DEBUG_ERROR,
@@ -662,7 +662,7 @@ SnpGetStatus (
   LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
 
   // Update the media status
-  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, LanDriver->PhyAddress,
+  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
                &phy_link_status);
   if (ogma_err != OGMA_ERR_OK) {
     DEBUG ((DEBUG_ERROR,
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
index 870833c8d31c..c95ff215199d 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
@@ -70,8 +70,6 @@ typedef struct {
   NON_DISCOVERABLE_DEVICE           *Dev;
 
   NETSEC_DEVICE_PATH                DevicePath;
-
-  UINTN                             PhyAddress;
 } NETSEC_DRIVER;
 
 #define NETSEC_SIGNATURE            SIGNATURE_32('n', 't', 's', 'c')
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
index 66f39150430b..be80dd9ae1fd 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
@@ -318,6 +318,7 @@ struct ogma_param_s{
     ogma_desc_ring_param_t desc_ring_param[OGMA_DESC_RING_ID_MAX+1];
     ogma_gmac_config_t gmac_config;
     ogma_uint8 mac_addr[6];
+    ogma_uint8 phy_addr;
 };
 
 struct ogma_tx_pkt_ctrl_s{
@@ -412,14 +413,12 @@ ogma_err_t ogma_set_gmac_mode (
 
 void ogma_set_phy_reg (
     ogma_handle_t ogma_handle,
-    ogma_uint8 phy_addr,
     ogma_uint8 reg_addr,
     ogma_uint16 value
     );
 
 ogma_uint16 ogma_get_phy_reg (
     ogma_handle_t ogma_handle,
-    ogma_uint8 phy_addr,
     ogma_uint8 reg_addr
     );
 
@@ -660,7 +659,6 @@ ogma_err_t ogma_get_gmac_lpitimer_reg (
 
 void ogma_set_phy_mmd_reg (
     ogma_handle_t ogma_handle,
-    ogma_uint8 phy_addr,
     ogma_uint8 dev_addr,
     ogma_uint16 reg_addr,
     ogma_uint16 value
@@ -668,14 +666,12 @@ void ogma_set_phy_mmd_reg (
 
 ogma_uint16 ogma_get_phy_mmd_reg (
     ogma_handle_t ogma_handle,
-    ogma_uint8 phy_addr,
     ogma_uint8 dev_addr,
     ogma_uint16 reg_addr
     );
 
 ogma_err_t ogma_get_phy_link_status (
     ogma_handle_t ogma_handle,
-    ogma_uint8 phy_addr,
     ogma_phy_link_status_t *phy_link_status_p
     );
 
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_gmac_access.c b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_gmac_access.c
index 88c149c10466..150d25ac3fbf 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_gmac_access.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_gmac_access.c
@@ -40,14 +40,12 @@
  **********************************************************************/
 static void ogma_set_phy_reg_sub (
     ogma_ctrl_t *ctrl_p,
-    ogma_uint8 phy_addr,
     ogma_uint8 reg_addr,
     ogma_uint16 value
     );
 
 static ogma_uint16 ogma_get_phy_reg_sub (
     ogma_ctrl_t *ctrl_p,
-    ogma_uint8 phy_addr,
     ogma_uint8 reg_addr
     );
 
@@ -57,14 +55,12 @@ void ogma_dump_gmac_stat (ogma_ctrl_t *ctrl_p);
 
 static void ogma_set_phy_target_mmd_reg_addr (
     ogma_ctrl_t *ctrl_p,
-    ogma_uint8 phy_addr,
     ogma_uint8 dev_addr,
     ogma_uint16 reg_addr
     );
 
 static void ogma_set_phy_mmd_reg_sub (
     ogma_ctrl_t *ctrl_p,
-    ogma_uint8 phy_addr,
     ogma_uint8 dev_addr,
     ogma_uint16 reg_addr,
     ogma_uint16 value
@@ -72,7 +68,6 @@ static void ogma_set_phy_mmd_reg_sub (
 
 static ogma_uint16 ogma_get_phy_mmd_reg_sub (
     ogma_ctrl_t *ctrl_p,
-    ogma_uint8 phy_addr,
     ogma_uint8 dev_addr,
     ogma_uint16 reg_addr
     );
@@ -435,7 +430,6 @@ ogma_err_t ogma_set_gmac_mode (
 
 static void ogma_set_phy_reg_sub (
     ogma_ctrl_t *ctrl_p,
-    ogma_uint8 phy_addr,
     ogma_uint8 reg_addr,
     ogma_uint16 value
     )
@@ -447,7 +441,7 @@ static void ogma_set_phy_reg_sub (
                       OGMA_GMAC_REG_ADDR_GDR,
                       value);
 
-    cmd = ( ( phy_addr << OGMA_GMAC_GAR_REG_SHIFT_PA) |
+    cmd = ( ( ctrl_p->param.phy_addr << OGMA_GMAC_GAR_REG_SHIFT_PA) |
             ( reg_addr << OGMA_GMAC_GAR_REG_SHIFT_GR) |
             ( OGMA_CLOCK_RANGE_IDX << OGMA_GMAC_GAR_REG_SHIFT_CR) |
             OGMA_GMAC_GAR_REG_GW |
@@ -466,7 +460,6 @@ static void ogma_set_phy_reg_sub (
 
 void ogma_set_phy_reg (
     ogma_handle_t ogma_handle,
-    ogma_uint8 phy_addr,
     ogma_uint8 reg_addr,
     ogma_uint16 value
     )
@@ -476,27 +469,25 @@ void ogma_set_phy_reg (
 
     if (( ctrl_p == NULL)
         || ( !ctrl_p->param.use_gmac_flag)
-        || ( phy_addr >= 32)
         || ( reg_addr >= 32) ) {
         pfdep_print( PFDEP_DEBUG_LEVEL_FATAL,
                      "An error occurred at ogma_set_phy_reg.\nPlease set valid argument.\n");
         return;
     }
 
-    ogma_set_phy_reg_sub( ctrl_p, phy_addr, reg_addr, value);
+    ogma_set_phy_reg_sub( ctrl_p, reg_addr, value);
 
 }
 
 static ogma_uint16 ogma_get_phy_reg_sub (
     ogma_ctrl_t *ctrl_p,
-    ogma_uint8 phy_addr,
     ogma_uint8 reg_addr
     )
 {
 
     ogma_uint32 cmd;
 
-    cmd = ( ( phy_addr << OGMA_GMAC_GAR_REG_SHIFT_PA) |
+    cmd = ( ( ctrl_p->param.phy_addr << OGMA_GMAC_GAR_REG_SHIFT_PA) |
             ( reg_addr << OGMA_GMAC_GAR_REG_SHIFT_GR) |
             ( OGMA_CLOCK_RANGE_IDX << OGMA_GMAC_GAR_REG_SHIFT_CR) |
             OGMA_GMAC_GAR_REG_GB);
@@ -516,7 +507,6 @@ static ogma_uint16 ogma_get_phy_reg_sub (
 
 ogma_uint16 ogma_get_phy_reg (
     ogma_handle_t ogma_handle,
-    ogma_uint8 phy_addr,
     ogma_uint8 reg_addr
     )
 {
@@ -525,14 +515,13 @@ ogma_uint16 ogma_get_phy_reg (
 
     if ( ( ctrl_p == NULL)
          || ( !ctrl_p->param.use_gmac_flag)
-         || ( phy_addr >= 32)
          || ( reg_addr >= 32) ) {
         pfdep_print( PFDEP_DEBUG_LEVEL_FATAL,
                      "An error occurred at ogma_get_phy_reg.\nPlease set valid argument.\n");
         return 0;
     }
 
-    value = ogma_get_phy_reg_sub(ctrl_p, phy_addr, reg_addr);
+    value = ogma_get_phy_reg_sub(ctrl_p, reg_addr);
 
 
     return value;
@@ -702,7 +691,6 @@ ogma_err_t ogma_get_gmac_status (
 
 static void ogma_set_phy_target_mmd_reg_addr (
     ogma_ctrl_t *ctrl_p,
-    ogma_uint8 phy_addr,
     ogma_uint8 dev_addr,
     ogma_uint16 reg_addr
     )
@@ -713,21 +701,20 @@ static void ogma_set_phy_target_mmd_reg_addr (
     cmd = ( ogma_uint32)dev_addr;
 
     /*set command to MMD access control register */
-    ogma_set_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_MMD_AC, cmd);
+    ogma_set_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_MMD_AC, cmd);
 
     /* set MMD access address data register Write reg_addr */
-    ogma_set_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_MMD_AAD, reg_addr);
+    ogma_set_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_MMD_AAD, reg_addr);
 
     /* write value to MMD ADDR */
     cmd = ( (1U << 14) | dev_addr);
 
     /* set command to MMD access control register */
-    ogma_set_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_MMD_AC, cmd);
+    ogma_set_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_MMD_AC, cmd);
 }
 
 static void ogma_set_phy_mmd_reg_sub (
     ogma_ctrl_t *ctrl_p,
-    ogma_uint8 phy_addr,
     ogma_uint8 dev_addr,
     ogma_uint16 reg_addr,
     ogma_uint16 value
@@ -735,30 +722,27 @@ static void ogma_set_phy_mmd_reg_sub (
 {
     /* set target mmd reg_addr */
     ogma_set_phy_target_mmd_reg_addr( ctrl_p,
-                                      phy_addr,
                                       dev_addr,
                                       reg_addr);
 
     /* Write value to MMD access address data register */
-    ogma_set_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_MMD_AAD, value);
+    ogma_set_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_MMD_AAD, value);
 
 }
 
 static ogma_uint16 ogma_get_phy_mmd_reg_sub (
     ogma_ctrl_t *ctrl_p,
-    ogma_uint8 phy_addr,
     ogma_uint8 dev_addr,
     ogma_uint16 reg_addr
     )
 {
     /* set target mmd reg_addr */
     ogma_set_phy_target_mmd_reg_addr( ctrl_p,
-                                      phy_addr,
                                       dev_addr,
                                       reg_addr);
 
     /* Read value for MMD access address data register */
-    return ogma_get_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_MMD_AAD);
+    return ogma_get_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_MMD_AAD);
 }
 
 ogma_err_t ogma_set_gmac_lpictrl_reg (
@@ -878,7 +862,6 @@ ogma_err_t ogma_get_gmac_lpitimer_reg (
 
 void ogma_set_phy_mmd_reg (
     ogma_handle_t ogma_handle,
-    ogma_uint8 phy_addr,
     ogma_uint8 dev_addr,
     ogma_uint16 reg_addr,
     ogma_uint16 value
@@ -890,8 +873,7 @@ void ogma_set_phy_mmd_reg (
         return;
     }
 
-    if ( ( phy_addr > 31U) ||
-         ( dev_addr > 31U) ) {
+    if ( dev_addr > 31U) {
         return;
     }
 
@@ -900,7 +882,6 @@ void ogma_set_phy_mmd_reg (
     }
 
     ogma_set_phy_mmd_reg_sub ( ctrl_p,
-                               phy_addr,
                                dev_addr,
                                reg_addr,
                                value);
@@ -911,7 +892,6 @@ void ogma_set_phy_mmd_reg (
 
 ogma_uint16 ogma_get_phy_mmd_reg (
     ogma_handle_t ogma_handle,
-    ogma_uint8 phy_addr,
     ogma_uint8 dev_addr,
     ogma_uint16 reg_addr
     )
@@ -923,8 +903,7 @@ ogma_uint16 ogma_get_phy_mmd_reg (
         return 0;
     }
 
-    if ( ( phy_addr > 31U) ||
-         ( dev_addr > 31U) ) {
+    if ( dev_addr > 31U) {
         return 0;
     }
 
@@ -933,7 +912,6 @@ ogma_uint16 ogma_get_phy_mmd_reg (
     }
 
     value = ogma_get_phy_mmd_reg_sub ( ctrl_p,
-                                       phy_addr,
                                        dev_addr,
                                        reg_addr);
 
@@ -943,7 +921,6 @@ ogma_uint16 ogma_get_phy_mmd_reg (
 
 ogma_err_t ogma_get_phy_link_status (
     ogma_handle_t ogma_handle,
-    ogma_uint8 phy_addr,
     ogma_phy_link_status_t *phy_link_status_p
     )
 {
@@ -955,10 +932,6 @@ ogma_err_t ogma_get_phy_link_status (
         return OGMA_ERR_PARAM;
     }
 
-    if ( phy_addr >= 32) {
-        return OGMA_ERR_RANGE;
-    }
-
     if ( !ctrl_p->param.use_gmac_flag) {
         return OGMA_ERR_NOTAVAIL;
     }
@@ -966,17 +939,17 @@ ogma_err_t ogma_get_phy_link_status (
     pfdep_memset( phy_link_status_p, 0, sizeof( ogma_phy_link_status_t) );
 
     /* Read PHY CONTROL Register */
-    tmp = ogma_get_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_CONTROL);
+    tmp = ogma_get_phy_reg_sub( ctrl_p,  OGMA_PHY_REG_ADDR_CONTROL);
 
     /* Read PHY STATUS Register */
-    value = ogma_get_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_STATUS);
+    value = ogma_get_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_STATUS);
 
     /* check latched_link_down_flag */
     if ( ( value & ( 1U << OGMA_PHY_STATUS_REG_LINK_STATUS) ) == 0) {
         phy_link_status_p->latched_link_down_flag = OGMA_TRUE;
 
         /* Read PHY STATUS Register */
-        value = ogma_get_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_STATUS);
+        value = ogma_get_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_STATUS);
 
     }
 
@@ -1036,12 +1009,10 @@ ogma_err_t ogma_get_phy_link_status (
 
             /* Read MASTER-SLAVE Control Register */
             value = ogma_get_phy_reg_sub( ctrl_p,
-                                          phy_addr,
                                           OGMA_PHY_REG_ADDR_MASTER_SLAVE_CONTROL);
 
             /* Read MASTER-SLAVE Status Register */
             tmp = ogma_get_phy_reg_sub( ctrl_p,
-                                        phy_addr,
                                         OGMA_PHY_REG_ADDR_MASTER_SLAVE_STATUS);
 
             /* Check Current Link Speed */
@@ -1061,12 +1032,10 @@ ogma_err_t ogma_get_phy_link_status (
 
                 /* Read Auto-Negotiation Advertisement register */
                 value = ogma_get_phy_reg_sub( ctrl_p,
-                                              phy_addr,
                                               OGMA_PHY_REG_ADDR_AUTO_NEGO_ABILTY);
 
                 /* Read Auto-Negotiation Link Partner Base Page Ability register */
                 tmp = ogma_get_phy_reg_sub( ctrl_p,
-                                             phy_addr,
                                              OGMA_PHY_REG_ADDR_AUTO_NEGO_LINK_PATNER_ABILTY);
 
                 value = ( ( ( value & tmp) >> OGMA_PHY_ANA_REG_TAF) &
@@ -1109,13 +1078,11 @@ ogma_err_t ogma_get_phy_link_status (
 
         /* Read EEE advertisement register */
         value = ogma_get_phy_mmd_reg_sub( ctrl_p,
-                                          phy_addr,
                                           OGMA_PHY_DEV_ADDR_AUTO_NEGO,
                                           OGMA_PHY_AUTO_NEGO_REG_ADDR_EEE_ADVERTISE);
 
         /* Read EEE link partner ability register */
         tmp = ogma_get_phy_mmd_reg_sub( ctrl_p,
-                                        phy_addr,
                                         OGMA_PHY_DEV_ADDR_AUTO_NEGO,
                                         OGMA_PHY_AUTO_NEGO_REG_ADDR_EEE_LP_ABILITY);
 
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_internal.h b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_internal.h
index ed09a7ada85d..a7bc69cf0777 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_internal.h
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_internal.h
@@ -111,6 +111,8 @@ struct ogma_ctrl_s{
 
     pfdep_phys_addr_t dummy_desc_entry_phys_addr;
 
+    ogma_uint8 phy_addr;
+
 #ifdef OGMA_CONFIG_REC_STAT
     /**
      * Statistics information.
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
index 4dec66313aa1..7481d2da2d24 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
@@ -388,6 +388,12 @@ ogma_err_t ogma_init (
         return OGMA_ERR_DATA;
     }
 
+    if ( param_p->phy_addr >= 32) {
+        pfdep_print( PFDEP_DEBUG_LEVEL_FATAL,
+                     "Error: phy_addr out of range\n");
+        return OGMA_ERR_DATA;
+    }
+
     ogma_err = ogma_probe_hardware( base_addr);
 
     if ( ogma_err != OGMA_ERR_OK) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [edk2-platforms PATCH v1 2/3] NetsecDxe: put phy in loopback mode in order to guarantee stable RXCLK input
  2019-07-22 11:56 [edk2-platforms PATCH v1 0/3] Robust Netsec Initialiation Masahisa Kojima
  2019-07-22 11:56 ` [edk2-platforms PATCH v1 1/3] NetsecDxe: embed phy address into NETSEC SDK internal structure Masahisa Kojima
@ 2019-07-22 11:56 ` Masahisa Kojima
  2019-07-23 15:49   ` Leif Lindholm
  2019-07-22 11:56 ` [edk2-platforms PATCH v1 3/3] NetsecDxe: SnpInitialize() waits for media linking up Masahisa Kojima
  2 siblings, 1 reply; 8+ messages in thread
From: Masahisa Kojima @ 2019-07-22 11:56 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif.lindholm, okamoto.satoru, Masahisa Kojima

NETSEC hardware requires stable RXCLK input upon initialization
triggered with DISCORE = 0.
However, RXCLK input could be unstable depending on phy chipset
and deployed network environment, which could cause NETSEC to
hang up during initialization.

We solve this platform/environment dependent issue by temporarily
putting phy in loopback mode, then we can expect the stable RXCLK input.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Signed-off-by: Satoru Okamoto <okamoto.satoru@socionext.com>
---
 .../netsec_sdk/src/ogma_misc.c                | 72 ++++++++++++++++++-
 .../netsec_for_uefi/netsec_sdk/src/ogma_reg.h |  4 ++
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
index 7481d2da2d24..07308d38a5c2 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
@@ -35,7 +35,6 @@ static const ogma_uint32 desc_ring_config_reg_addr[OGMA_DESC_RING_ID_MAX + 1] =
 
 };
 
-
 /* Internal function definition*/
 #ifndef OGMA_CONFIG_DISABLE_CLK_CTRL
 STATIC void ogma_set_clk_en_reg (
@@ -327,6 +326,60 @@ STATIC ogma_uint32 ogma_calc_pkt_ctrl_reg_param (
     return param;
 }
 
+STATIC
+void
+ogma_pre_init_microengine (
+  ogma_handle_t ogma_handle
+  )
+{
+  UINT16 Data;
+
+  /* Remove dormant settings */
+  Data = ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
+    ~((1U << OGMA_PHY_CONTROL_REG_POWER_DOWN) |
+      (1U << OGMA_PHY_CONTROL_REG_ISOLATE));
+
+  ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data);
+
+  while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
+          ((1U << OGMA_PHY_CONTROL_REG_POWER_DOWN) |
+           (1U << OGMA_PHY_CONTROL_REG_ISOLATE))) != 0);
+
+  /* Put phy in loopback mode to guarantee RXCLK input */
+  Data |= (1U << OGMA_PHY_CONTROL_REG_LOOPBACK);
+  
+  ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data);
+
+  while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
+          (1U << OGMA_PHY_CONTROL_REG_LOOPBACK)) == 0);
+}
+
+STATIC
+void
+ogma_post_init_microengine (
+  IN ogma_handle_t ogma_handle
+  )
+{
+  UINT16 Data;
+
+  /* Get phy back to normal operation */
+  Data = ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
+    ~(1U << OGMA_PHY_CONTROL_REG_LOOPBACK);
+
+  ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data);
+
+  while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
+          (1U << OGMA_PHY_CONTROL_REG_LOOPBACK)) != 0);
+
+  Data |= (1U << OGMA_PHY_CONTROL_REG_RESET);
+  
+  /* Apply software reset */
+  ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data);
+
+  while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
+          (1U << OGMA_PHY_CONTROL_REG_RESET)) != 0);
+}
+
 ogma_err_t ogma_init (
     void *base_addr,
     pfdep_dev_handle_t dev_handle,
@@ -551,6 +604,16 @@ ogma_err_t ogma_init (
     ogma_write_reg( ctrl_p, OGMA_REG_ADDR_DMA_TMR_CTRL,
                     ( ogma_uint32)( ( OGMA_CONFIG_CLK_HZ / OGMA_CLK_MHZ) - 1) );
 
+    /*
+     * Do pre-initialization tasks for microengine
+     *
+     * In particular, we put phy in loopback mode
+     * in order to make sure RXCLK keeps provided to mac
+     * irrespective of phy link status,
+     * which is required for microengine intialization.
+     */
+    ogma_pre_init_microengine (ctrl_p);
+
     /* start microengines */
     ogma_write_reg( ctrl_p, OGMA_REG_ADDR_DIS_CORE, 0);
 
@@ -573,6 +636,13 @@ ogma_err_t ogma_init (
         goto err;
     }
 
+    /*
+     * Do post-initialization tasks for microengine
+     *
+     * We put phy in normal mode and apply reset.
+     */
+    ogma_post_init_microengine (ctrl_p);
+
     /* clear microcode load end status */
     ogma_write_reg( ctrl_p, OGMA_REG_ADDR_TOP_STATUS,
                     OGMA_TOP_IRQ_REG_ME_START);
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h
index 30c716352b37..ca769084cb31 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h
@@ -138,8 +138,12 @@
 /* bit fields for PHY CONTROL Register */
 #define OGMA_PHY_CONTROL_REG_SPEED_SELECTION_MSB (6)
 #define OGMA_PHY_CONTROL_REG_DUPLEX_MODE         (8)
+#define OGMA_PHY_CONTROL_REG_ISOLATE             (10)
+#define OGMA_PHY_CONTROL_REG_POWER_DOWN          (11)
 #define OGMA_PHY_CONTROL_REG_AUTO_NEGO_ENABLE    (12)
 #define OGMA_PHY_CONTROL_REG_SPEED_SELECTION_LSB (13)
+#define OGMA_PHY_CONTROL_REG_LOOPBACK            (14)
+#define OGMA_PHY_CONTROL_REG_RESET               (15)
 
 /* bit fields for PHY STATUS Register */
 #define OGMA_PHY_STATUS_REG_LINK_STATUS       (2)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [edk2-platforms PATCH v1 3/3] NetsecDxe: SnpInitialize() waits for media linking up
  2019-07-22 11:56 [edk2-platforms PATCH v1 0/3] Robust Netsec Initialiation Masahisa Kojima
  2019-07-22 11:56 ` [edk2-platforms PATCH v1 1/3] NetsecDxe: embed phy address into NETSEC SDK internal structure Masahisa Kojima
  2019-07-22 11:56 ` [edk2-platforms PATCH v1 2/3] NetsecDxe: put phy in loopback mode in order to guarantee stable RXCLK input Masahisa Kojima
@ 2019-07-22 11:56 ` Masahisa Kojima
  2019-07-23 16:14   ` Leif Lindholm
  2 siblings, 1 reply; 8+ messages in thread
From: Masahisa Kojima @ 2019-07-22 11:56 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif.lindholm, okamoto.satoru, Masahisa Kojima

The latest NetsecDxe requires issueing phy reset at the
last stage of initialization to safely exit loopback mode.
However, as a result, it takes a couple of seconds for link state
to get stable, which could cause auto-chosen pxeboot to fail
due to MediaPresent check error.

This patch adds link state check with 5s timeout in NetsecDxe
initialization. The timeout value can be adjustable via
configuration file.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Signed-off-by: Satoru Okamoto <okamoto.satoru@socionext.com>
---
 .../Socionext/DeveloperBox/DeveloperBox.dsc   |   1 +
 .../Drivers/Net/NetsecDxe/NetsecDxe.c         | 232 ++++++++----------
 .../Drivers/Net/NetsecDxe/NetsecDxe.dec       |   1 +
 .../Drivers/Net/NetsecDxe/NetsecDxe.inf       |   1 +
 4 files changed, 110 insertions(+), 125 deletions(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index 97fb8c410c60..af149607f3ce 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -138,6 +138,7 @@
   gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStartThreshold|36
   gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStopThreshold|48
   gNetsecDxeTokenSpaceGuid.PcdPauseTime|256
+  gNetsecDxeTokenSpaceGuid.PcdMediaDetectTimeoutOnBoot|5
 
   gSynQuacerTokenSpaceGuid.PcdNetsecEepromBase|0x08080000
   gSynQuacerTokenSpaceGuid.PcdNetsecPhyAddress|7
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
index 0b91d4af44a3..a304e02208fa 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
@@ -169,6 +169,98 @@ ExitUnlock:
   return Status;
 }
 
+EFI_STATUS
+EFIAPI
+NetsecUpdateLink (
+  IN  EFI_SIMPLE_NETWORK_PROTOCOL    *Snp
+  )
+{
+  NETSEC_DRIVER                 *LanDriver;
+  ogma_phy_link_status_t        phy_link_status;
+  ogma_gmac_mode_t              ogma_gmac_mode;
+  ogma_err_t                    ogma_err;
+  BOOLEAN                       ValidFlag;
+  ogma_gmac_mode_t              GmacMode;
+  BOOLEAN                       RxRunningFlag;
+  BOOLEAN                       TxRunningFlag;
+  EFI_STATUS                    ErrorStatus;
+
+  LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
+
+  // Update the media status
+  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
+               &phy_link_status);
+  if (ogma_err != OGMA_ERR_OK) {
+    DEBUG ((DEBUG_ERROR,
+      "NETSEC: ogma_get_phy_link_status failed with error code: %d\n",
+      (INT32)ogma_err));
+    ErrorStatus = EFI_DEVICE_ERROR;
+    goto Fail;
+  }
+
+  // Update the GMAC status
+  ogma_err = ogma_get_gmac_status (LanDriver->Handle, &ValidFlag, &GmacMode,
+               &RxRunningFlag, &TxRunningFlag);
+  if (ogma_err != OGMA_ERR_OK) {
+    DEBUG ((DEBUG_ERROR,
+      "NETSEC: ogma_get_gmac_status failed with error code: %d\n",
+      (INT32)ogma_err));
+    ErrorStatus = EFI_DEVICE_ERROR;
+    goto Fail;
+  }
+
+  // Stop GMAC when GMAC is running and physical link is down
+  if (RxRunningFlag && TxRunningFlag && !phy_link_status.up_flag) {
+    ogma_err = ogma_stop_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
+    if (ogma_err != OGMA_ERR_OK) {
+      DEBUG ((DEBUG_ERROR,
+        "NETSEC: ogma_stop_gmac() failed with error status %d\n",
+        ogma_err));
+      ErrorStatus = EFI_DEVICE_ERROR;
+      goto Fail;
+    }
+  }
+
+  // Start GMAC when GMAC is stopped and physical link is up
+  if (!RxRunningFlag && !TxRunningFlag && phy_link_status.up_flag) {
+    ZeroMem (&ogma_gmac_mode, sizeof (ogma_gmac_mode_t));
+    ogma_gmac_mode.link_speed = phy_link_status.link_speed;
+    ogma_gmac_mode.half_duplex_flag = (ogma_bool)phy_link_status.half_duplex_flag;
+    if (!phy_link_status.half_duplex_flag && FixedPcdGet8 (PcdFlowCtrl)) {
+      ogma_gmac_mode.flow_ctrl_enable_flag     = FixedPcdGet8 (PcdFlowCtrl);
+      ogma_gmac_mode.flow_ctrl_start_threshold = FixedPcdGet16 (PcdFlowCtrlStartThreshold);
+      ogma_gmac_mode.flow_ctrl_stop_threshold  = FixedPcdGet16 (PcdFlowCtrlStopThreshold);
+      ogma_gmac_mode.pause_time                = FixedPcdGet16 (PcdPauseTime);
+    }
+
+    ogma_err = ogma_set_gmac_mode (LanDriver->Handle, &ogma_gmac_mode);
+    if (ogma_err != OGMA_ERR_OK) {
+      DEBUG ((DEBUG_ERROR,
+        "NETSEC: ogma_set_gmac() failed with error status %d\n",
+        (INT32)ogma_err));
+      ErrorStatus = EFI_DEVICE_ERROR;
+      goto Fail;
+    }
+
+    ogma_err = ogma_start_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
+    if (ogma_err != OGMA_ERR_OK) {
+      DEBUG ((DEBUG_ERROR,
+        "NETSEC: ogma_start_gmac() failed with error status %d\n",
+        (INT32)ogma_err));
+      ErrorStatus = EFI_DEVICE_ERROR;
+      goto Fail;
+    }
+  }
+
+  /* Updating link status for external guery */
+  Snp->Mode->MediaPresent = phy_link_status.up_flag;
+  return EFI_SUCCESS;
+
+Fail:
+  Snp->Mode->MediaPresent = FALSE;
+  return ErrorStatus;
+}
+
 /*
  *  UEFI Initialize() function
  */
@@ -185,9 +277,9 @@ SnpInitialize (
   EFI_TPL                 SavedTpl;
   EFI_STATUS              Status;
 
-  ogma_phy_link_status_t  phy_link_status;
   ogma_err_t              ogma_err;
-  ogma_gmac_mode_t        ogma_gmac_mode;
+
+  UINT32                  Index;
 
   // Check Snp Instance
   if (Snp == NULL) {
@@ -271,48 +363,18 @@ SnpInitialize (
   ogma_disable_desc_ring_irq (LanDriver->Handle, OGMA_DESC_RING_ID_NRM_TX,
                               OGMA_CH_IRQ_REG_EMPTY);
 
-  // Stop and restart the physical link
-  ogma_err = ogma_stop_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
-  if (ogma_err != OGMA_ERR_OK) {
-    DEBUG ((DEBUG_ERROR,
-      "NETSEC: ogma_stop_gmac() failed with error status %d\n",
-      ogma_err));
-    ReturnUnlock (EFI_DEVICE_ERROR);
-  }
-
-  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
-               &phy_link_status);
-  if (ogma_err != OGMA_ERR_OK) {
-    DEBUG ((DEBUG_ERROR,
-      "NETSEC: ogma_get_phy_link_status() failed error code %d\n",
-      (INT32)ogma_err));
-    ReturnUnlock (EFI_DEVICE_ERROR);
-  }
-
-  SetMem (&ogma_gmac_mode, sizeof (ogma_gmac_mode_t), 0);
-  ogma_gmac_mode.link_speed = phy_link_status.link_speed;
-  ogma_gmac_mode.half_duplex_flag = (ogma_bool)phy_link_status.half_duplex_flag;
-  if ((!phy_link_status.half_duplex_flag) && FixedPcdGet8 (PcdFlowCtrl)) {
-    ogma_gmac_mode.flow_ctrl_enable_flag     = FixedPcdGet8 (PcdFlowCtrl);
-    ogma_gmac_mode.flow_ctrl_start_threshold = FixedPcdGet16 (PcdFlowCtrlStartThreshold);
-    ogma_gmac_mode.flow_ctrl_stop_threshold  = FixedPcdGet16 (PcdFlowCtrlStopThreshold);
-    ogma_gmac_mode.pause_time                = FixedPcdGet16 (PcdPauseTime);
-  }
-
-  ogma_err = ogma_set_gmac_mode (LanDriver->Handle, &ogma_gmac_mode);
-  if (ogma_err != OGMA_ERR_OK) {
-    DEBUG ((DEBUG_ERROR,
-      "NETSEC: ogma_set_gmac() failed with error status %d\n",
-      (INT32)ogma_err));
-    ReturnUnlock (EFI_DEVICE_ERROR);
-  }
-
-  ogma_err = ogma_start_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
-  if (ogma_err != OGMA_ERR_OK) {
-    DEBUG ((DEBUG_ERROR,
-      "NETSEC: ogma_start_gmac() failed with error status %d\n",
-      (INT32)ogma_err));
-    ReturnUnlock (EFI_DEVICE_ERROR);
+  // Wait for media linking up
+  for (Index = 0; Index < (UINT32)FixedPcdGet8 (PcdMediaDetectTimeoutOnBoot) * 10; Index++) {
+    Status = NetsecUpdateLink (Snp);
+    if (Status != EFI_SUCCESS) {
+      ReturnUnlock (EFI_DEVICE_ERROR);
+    }
+
+    if (Snp->Mode->MediaPresent) {
+      break;
+    }
+
+    MicroSecondDelay(100000);
   }
 
   // Declare the driver as initialized
@@ -420,14 +482,6 @@ NetsecPollPhyStatus (
   )
 {
   EFI_SIMPLE_NETWORK_PROTOCOL   *Snp;
-  NETSEC_DRIVER                 *LanDriver;
-  ogma_phy_link_status_t        phy_link_status;
-  ogma_gmac_mode_t              ogma_gmac_mode;
-  ogma_err_t                    ogma_err;
-  BOOLEAN                       ValidFlag;
-  ogma_gmac_mode_t              GmacMode;
-  BOOLEAN                       RxRunningFlag;
-  BOOLEAN                       TxRunningFlag;
 
   Snp = (EFI_SIMPLE_NETWORK_PROTOCOL *)Context;
   if (Snp == NULL) {
@@ -435,66 +489,7 @@ NetsecPollPhyStatus (
     return;
   }
 
-  LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
-
-  // Update the media status
-  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
-               &phy_link_status);
-  if (ogma_err != OGMA_ERR_OK) {
-    DEBUG ((DEBUG_ERROR,
-      "NETSEC: ogma_get_phy_link_status failed with error code: %d\n",
-      (INT32)ogma_err));
-    return;
-  }
-
-  // Update the GMAC status
-  ogma_err = ogma_get_gmac_status (LanDriver->Handle, &ValidFlag, &GmacMode,
-               &RxRunningFlag, &TxRunningFlag);
-  if (ogma_err != OGMA_ERR_OK) {
-    DEBUG ((DEBUG_ERROR,
-      "NETSEC: ogma_get_gmac_status failed with error code: %d\n",
-      (INT32)ogma_err));
-    return;
-  }
-
-  // Stop GMAC when GMAC is running and physical link is down
-  if (RxRunningFlag && TxRunningFlag && !phy_link_status.up_flag) {
-    ogma_err = ogma_stop_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
-    if (ogma_err != OGMA_ERR_OK) {
-      DEBUG ((DEBUG_ERROR,
-        "NETSEC: ogma_stop_gmac() failed with error status %d\n",
-        ogma_err));
-      return;
-    }
-  }
-
-  // Start GMAC when GMAC is stopped and physical link is up
-  if (!RxRunningFlag && !TxRunningFlag && phy_link_status.up_flag) {
-    ZeroMem (&ogma_gmac_mode, sizeof (ogma_gmac_mode_t));
-    ogma_gmac_mode.link_speed = phy_link_status.link_speed;
-    ogma_gmac_mode.half_duplex_flag = (ogma_bool)phy_link_status.half_duplex_flag;
-    if (!phy_link_status.half_duplex_flag && FixedPcdGet8 (PcdFlowCtrl)) {
-      ogma_gmac_mode.flow_ctrl_enable_flag     = FixedPcdGet8 (PcdFlowCtrl);
-      ogma_gmac_mode.flow_ctrl_start_threshold = FixedPcdGet16 (PcdFlowCtrlStartThreshold);
-      ogma_gmac_mode.flow_ctrl_stop_threshold  = FixedPcdGet16 (PcdFlowCtrlStopThreshold);
-      ogma_gmac_mode.pause_time                = FixedPcdGet16 (PcdPauseTime);
-    }
-
-    ogma_err = ogma_set_gmac_mode (LanDriver->Handle, &ogma_gmac_mode);
-    if (ogma_err != OGMA_ERR_OK) {
-      DEBUG ((DEBUG_ERROR,
-        "NETSEC: ogma_set_gmac() failed with error status %d\n",
-        (INT32)ogma_err));
-      return;
-    }
-
-    ogma_err = ogma_start_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
-    if (ogma_err != OGMA_ERR_OK) {
-      DEBUG ((DEBUG_ERROR,
-        "NETSEC: ogma_start_gmac() failed with error status %d\n",
-        (INT32)ogma_err));
-    }
-  }
+  NetsecUpdateLink (Snp);
 }
 
 /*
@@ -631,7 +626,6 @@ SnpGetStatus (
   pfdep_pkt_handle_t        pkt_handle;
   LIST_ENTRY                *Link;
 
-  ogma_phy_link_status_t  phy_link_status;
   ogma_err_t              ogma_err;
 
   // Check preliminaries
@@ -661,18 +655,6 @@ SnpGetStatus (
   // Find the LanDriver structure
   LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
 
-  // Update the media status
-  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
-               &phy_link_status);
-  if (ogma_err != OGMA_ERR_OK) {
-    DEBUG ((DEBUG_ERROR,
-      "NETSEC: ogma_get_phy_link_status failed with error code: %d\n",
-      (INT32)ogma_err));
-    ReturnUnlock (EFI_DEVICE_ERROR);
-  }
-
-  Snp->Mode->MediaPresent = phy_link_status.up_flag;
-
   ogma_err = ogma_clean_tx_desc_ring (LanDriver->Handle,
                                       OGMA_DESC_RING_ID_NRM_TX);
 
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec
index 6b9f60293879..016eba5ef695 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec
@@ -38,3 +38,4 @@
   gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStartThreshold|0x0|UINT16|0x00000006
   gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStopThreshold|0x0|UINT16|0x00000007
   gNetsecDxeTokenSpaceGuid.PcdPauseTime|0x0|UINT16|0x00000008
+  gNetsecDxeTokenSpaceGuid.PcdMediaDetectTimeoutOnBoot|0x0|UINT8|0x00000009
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf
index 49dd28efc65b..818014e6d257 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf
@@ -62,3 +62,4 @@
   gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStopThreshold
   gNetsecDxeTokenSpaceGuid.PcdJumboPacket
   gNetsecDxeTokenSpaceGuid.PcdPauseTime
+  gNetsecDxeTokenSpaceGuid.PcdMediaDetectTimeoutOnBoot
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [edk2-platforms PATCH v1 1/3] NetsecDxe: embed phy address into NETSEC SDK internal structure
  2019-07-22 11:56 ` [edk2-platforms PATCH v1 1/3] NetsecDxe: embed phy address into NETSEC SDK internal structure Masahisa Kojima
@ 2019-07-23 15:41   ` Leif Lindholm
  0 siblings, 0 replies; 8+ messages in thread
From: Leif Lindholm @ 2019-07-23 15:41 UTC (permalink / raw)
  To: Masahisa Kojima; +Cc: devel, ard.biesheuvel, okamoto.satoru

On Mon, Jul 22, 2019 at 08:56:34PM +0900, Masahisa Kojima wrote:
> This is a refactoring of phy address handling in Netsec driver.
> NETSEC SDK, low level driver for NetsecDxe, did not store phy address.
> User should specify the phy address as an argument to
> the SDK public functions.
> It prevented NETSEC SDK from internally controlling phy,
> and it also bothers user application with phy address management.
> 
> With that, we encapsulate the phy address into NETSEC SDK.
> 
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> Signed-off-by: Satoru Okamoto <okamoto.satoru@socionext.com>
> ---
>  .../Drivers/Net/NetsecDxe/NetsecDxe.c         | 10 +--
>  .../Drivers/Net/NetsecDxe/NetsecDxe.h         |  2 -
>  .../netsec_sdk/include/ogma_api.h             |  6 +-
>  .../netsec_sdk/src/ogma_gmac_access.c         | 61 +++++--------------
>  .../netsec_sdk/src/ogma_internal.h            |  2 +
>  .../netsec_sdk/src/ogma_misc.c                |  6 ++

Please use --stat=1000 when generating patches, to avoid truncating
filenames in the summary.

Other than that, looks like good cleanup:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


>  6 files changed, 28 insertions(+), 59 deletions(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> index 160bb08a4632..0b91d4af44a3 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> @@ -59,6 +59,8 @@ Probe (
>    // phy-interface
>    Param.gmac_config.phy_interface = OGMA_PHY_INTERFACE_RGMII;
>  
> +  Param.phy_addr = LanDriver->Dev->Resources[2].AddrRangeMin;
> +
>    // Read and save the Permanent MAC Address
>    EepromBase = LanDriver->Dev->Resources[1].AddrRangeMin;
>    GetCurrentMacAddress (EepromBase, LanDriver->SnpMode.PermanentAddress.Addr);
> @@ -107,8 +109,6 @@ Probe (
>      return EFI_DEVICE_ERROR;
>    }
>  
> -  LanDriver->PhyAddress = LanDriver->Dev->Resources[2].AddrRangeMin;
> -
>    ogma_enable_top_irq (LanDriver->Handle,
>                         OGMA_TOP_IRQ_REG_NRM_RX | OGMA_TOP_IRQ_REG_NRM_TX);
>  
> @@ -280,7 +280,7 @@ SnpInitialize (
>      ReturnUnlock (EFI_DEVICE_ERROR);
>    }
>  
> -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, LanDriver->PhyAddress,
> +  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
>                 &phy_link_status);
>    if (ogma_err != OGMA_ERR_OK) {
>      DEBUG ((DEBUG_ERROR,
> @@ -438,7 +438,7 @@ NetsecPollPhyStatus (
>    LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
>  
>    // Update the media status
> -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, LanDriver->PhyAddress,
> +  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
>                 &phy_link_status);
>    if (ogma_err != OGMA_ERR_OK) {
>      DEBUG ((DEBUG_ERROR,
> @@ -662,7 +662,7 @@ SnpGetStatus (
>    LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
>  
>    // Update the media status
> -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle, LanDriver->PhyAddress,
> +  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
>                 &phy_link_status);
>    if (ogma_err != OGMA_ERR_OK) {
>      DEBUG ((DEBUG_ERROR,
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
> index 870833c8d31c..c95ff215199d 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
> @@ -70,8 +70,6 @@ typedef struct {
>    NON_DISCOVERABLE_DEVICE           *Dev;
>  
>    NETSEC_DEVICE_PATH                DevicePath;
> -
> -  UINTN                             PhyAddress;
>  } NETSEC_DRIVER;
>  
>  #define NETSEC_SIGNATURE            SIGNATURE_32('n', 't', 's', 'c')
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
> index 66f39150430b..be80dd9ae1fd 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/include/ogma_api.h
> @@ -318,6 +318,7 @@ struct ogma_param_s{
>      ogma_desc_ring_param_t desc_ring_param[OGMA_DESC_RING_ID_MAX+1];
>      ogma_gmac_config_t gmac_config;
>      ogma_uint8 mac_addr[6];
> +    ogma_uint8 phy_addr;
>  };
>  
>  struct ogma_tx_pkt_ctrl_s{
> @@ -412,14 +413,12 @@ ogma_err_t ogma_set_gmac_mode (
>  
>  void ogma_set_phy_reg (
>      ogma_handle_t ogma_handle,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 reg_addr,
>      ogma_uint16 value
>      );
>  
>  ogma_uint16 ogma_get_phy_reg (
>      ogma_handle_t ogma_handle,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 reg_addr
>      );
>  
> @@ -660,7 +659,6 @@ ogma_err_t ogma_get_gmac_lpitimer_reg (
>  
>  void ogma_set_phy_mmd_reg (
>      ogma_handle_t ogma_handle,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 dev_addr,
>      ogma_uint16 reg_addr,
>      ogma_uint16 value
> @@ -668,14 +666,12 @@ void ogma_set_phy_mmd_reg (
>  
>  ogma_uint16 ogma_get_phy_mmd_reg (
>      ogma_handle_t ogma_handle,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 dev_addr,
>      ogma_uint16 reg_addr
>      );
>  
>  ogma_err_t ogma_get_phy_link_status (
>      ogma_handle_t ogma_handle,
> -    ogma_uint8 phy_addr,
>      ogma_phy_link_status_t *phy_link_status_p
>      );
>  
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_gmac_access.c b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_gmac_access.c
> index 88c149c10466..150d25ac3fbf 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_gmac_access.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_gmac_access.c
> @@ -40,14 +40,12 @@
>   **********************************************************************/
>  static void ogma_set_phy_reg_sub (
>      ogma_ctrl_t *ctrl_p,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 reg_addr,
>      ogma_uint16 value
>      );
>  
>  static ogma_uint16 ogma_get_phy_reg_sub (
>      ogma_ctrl_t *ctrl_p,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 reg_addr
>      );
>  
> @@ -57,14 +55,12 @@ void ogma_dump_gmac_stat (ogma_ctrl_t *ctrl_p);
>  
>  static void ogma_set_phy_target_mmd_reg_addr (
>      ogma_ctrl_t *ctrl_p,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 dev_addr,
>      ogma_uint16 reg_addr
>      );
>  
>  static void ogma_set_phy_mmd_reg_sub (
>      ogma_ctrl_t *ctrl_p,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 dev_addr,
>      ogma_uint16 reg_addr,
>      ogma_uint16 value
> @@ -72,7 +68,6 @@ static void ogma_set_phy_mmd_reg_sub (
>  
>  static ogma_uint16 ogma_get_phy_mmd_reg_sub (
>      ogma_ctrl_t *ctrl_p,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 dev_addr,
>      ogma_uint16 reg_addr
>      );
> @@ -435,7 +430,6 @@ ogma_err_t ogma_set_gmac_mode (
>  
>  static void ogma_set_phy_reg_sub (
>      ogma_ctrl_t *ctrl_p,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 reg_addr,
>      ogma_uint16 value
>      )
> @@ -447,7 +441,7 @@ static void ogma_set_phy_reg_sub (
>                        OGMA_GMAC_REG_ADDR_GDR,
>                        value);
>  
> -    cmd = ( ( phy_addr << OGMA_GMAC_GAR_REG_SHIFT_PA) |
> +    cmd = ( ( ctrl_p->param.phy_addr << OGMA_GMAC_GAR_REG_SHIFT_PA) |
>              ( reg_addr << OGMA_GMAC_GAR_REG_SHIFT_GR) |
>              ( OGMA_CLOCK_RANGE_IDX << OGMA_GMAC_GAR_REG_SHIFT_CR) |
>              OGMA_GMAC_GAR_REG_GW |
> @@ -466,7 +460,6 @@ static void ogma_set_phy_reg_sub (
>  
>  void ogma_set_phy_reg (
>      ogma_handle_t ogma_handle,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 reg_addr,
>      ogma_uint16 value
>      )
> @@ -476,27 +469,25 @@ void ogma_set_phy_reg (
>  
>      if (( ctrl_p == NULL)
>          || ( !ctrl_p->param.use_gmac_flag)
> -        || ( phy_addr >= 32)
>          || ( reg_addr >= 32) ) {
>          pfdep_print( PFDEP_DEBUG_LEVEL_FATAL,
>                       "An error occurred at ogma_set_phy_reg.\nPlease set valid argument.\n");
>          return;
>      }
>  
> -    ogma_set_phy_reg_sub( ctrl_p, phy_addr, reg_addr, value);
> +    ogma_set_phy_reg_sub( ctrl_p, reg_addr, value);
>  
>  }
>  
>  static ogma_uint16 ogma_get_phy_reg_sub (
>      ogma_ctrl_t *ctrl_p,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 reg_addr
>      )
>  {
>  
>      ogma_uint32 cmd;
>  
> -    cmd = ( ( phy_addr << OGMA_GMAC_GAR_REG_SHIFT_PA) |
> +    cmd = ( ( ctrl_p->param.phy_addr << OGMA_GMAC_GAR_REG_SHIFT_PA) |
>              ( reg_addr << OGMA_GMAC_GAR_REG_SHIFT_GR) |
>              ( OGMA_CLOCK_RANGE_IDX << OGMA_GMAC_GAR_REG_SHIFT_CR) |
>              OGMA_GMAC_GAR_REG_GB);
> @@ -516,7 +507,6 @@ static ogma_uint16 ogma_get_phy_reg_sub (
>  
>  ogma_uint16 ogma_get_phy_reg (
>      ogma_handle_t ogma_handle,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 reg_addr
>      )
>  {
> @@ -525,14 +515,13 @@ ogma_uint16 ogma_get_phy_reg (
>  
>      if ( ( ctrl_p == NULL)
>           || ( !ctrl_p->param.use_gmac_flag)
> -         || ( phy_addr >= 32)
>           || ( reg_addr >= 32) ) {
>          pfdep_print( PFDEP_DEBUG_LEVEL_FATAL,
>                       "An error occurred at ogma_get_phy_reg.\nPlease set valid argument.\n");
>          return 0;
>      }
>  
> -    value = ogma_get_phy_reg_sub(ctrl_p, phy_addr, reg_addr);
> +    value = ogma_get_phy_reg_sub(ctrl_p, reg_addr);
>  
>  
>      return value;
> @@ -702,7 +691,6 @@ ogma_err_t ogma_get_gmac_status (
>  
>  static void ogma_set_phy_target_mmd_reg_addr (
>      ogma_ctrl_t *ctrl_p,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 dev_addr,
>      ogma_uint16 reg_addr
>      )
> @@ -713,21 +701,20 @@ static void ogma_set_phy_target_mmd_reg_addr (
>      cmd = ( ogma_uint32)dev_addr;
>  
>      /*set command to MMD access control register */
> -    ogma_set_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_MMD_AC, cmd);
> +    ogma_set_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_MMD_AC, cmd);
>  
>      /* set MMD access address data register Write reg_addr */
> -    ogma_set_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_MMD_AAD, reg_addr);
> +    ogma_set_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_MMD_AAD, reg_addr);
>  
>      /* write value to MMD ADDR */
>      cmd = ( (1U << 14) | dev_addr);
>  
>      /* set command to MMD access control register */
> -    ogma_set_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_MMD_AC, cmd);
> +    ogma_set_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_MMD_AC, cmd);
>  }
>  
>  static void ogma_set_phy_mmd_reg_sub (
>      ogma_ctrl_t *ctrl_p,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 dev_addr,
>      ogma_uint16 reg_addr,
>      ogma_uint16 value
> @@ -735,30 +722,27 @@ static void ogma_set_phy_mmd_reg_sub (
>  {
>      /* set target mmd reg_addr */
>      ogma_set_phy_target_mmd_reg_addr( ctrl_p,
> -                                      phy_addr,
>                                        dev_addr,
>                                        reg_addr);
>  
>      /* Write value to MMD access address data register */
> -    ogma_set_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_MMD_AAD, value);
> +    ogma_set_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_MMD_AAD, value);
>  
>  }
>  
>  static ogma_uint16 ogma_get_phy_mmd_reg_sub (
>      ogma_ctrl_t *ctrl_p,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 dev_addr,
>      ogma_uint16 reg_addr
>      )
>  {
>      /* set target mmd reg_addr */
>      ogma_set_phy_target_mmd_reg_addr( ctrl_p,
> -                                      phy_addr,
>                                        dev_addr,
>                                        reg_addr);
>  
>      /* Read value for MMD access address data register */
> -    return ogma_get_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_MMD_AAD);
> +    return ogma_get_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_MMD_AAD);
>  }
>  
>  ogma_err_t ogma_set_gmac_lpictrl_reg (
> @@ -878,7 +862,6 @@ ogma_err_t ogma_get_gmac_lpitimer_reg (
>  
>  void ogma_set_phy_mmd_reg (
>      ogma_handle_t ogma_handle,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 dev_addr,
>      ogma_uint16 reg_addr,
>      ogma_uint16 value
> @@ -890,8 +873,7 @@ void ogma_set_phy_mmd_reg (
>          return;
>      }
>  
> -    if ( ( phy_addr > 31U) ||
> -         ( dev_addr > 31U) ) {
> +    if ( dev_addr > 31U) {
>          return;
>      }
>  
> @@ -900,7 +882,6 @@ void ogma_set_phy_mmd_reg (
>      }
>  
>      ogma_set_phy_mmd_reg_sub ( ctrl_p,
> -                               phy_addr,
>                                 dev_addr,
>                                 reg_addr,
>                                 value);
> @@ -911,7 +892,6 @@ void ogma_set_phy_mmd_reg (
>  
>  ogma_uint16 ogma_get_phy_mmd_reg (
>      ogma_handle_t ogma_handle,
> -    ogma_uint8 phy_addr,
>      ogma_uint8 dev_addr,
>      ogma_uint16 reg_addr
>      )
> @@ -923,8 +903,7 @@ ogma_uint16 ogma_get_phy_mmd_reg (
>          return 0;
>      }
>  
> -    if ( ( phy_addr > 31U) ||
> -         ( dev_addr > 31U) ) {
> +    if ( dev_addr > 31U) {
>          return 0;
>      }
>  
> @@ -933,7 +912,6 @@ ogma_uint16 ogma_get_phy_mmd_reg (
>      }
>  
>      value = ogma_get_phy_mmd_reg_sub ( ctrl_p,
> -                                       phy_addr,
>                                         dev_addr,
>                                         reg_addr);
>  
> @@ -943,7 +921,6 @@ ogma_uint16 ogma_get_phy_mmd_reg (
>  
>  ogma_err_t ogma_get_phy_link_status (
>      ogma_handle_t ogma_handle,
> -    ogma_uint8 phy_addr,
>      ogma_phy_link_status_t *phy_link_status_p
>      )
>  {
> @@ -955,10 +932,6 @@ ogma_err_t ogma_get_phy_link_status (
>          return OGMA_ERR_PARAM;
>      }
>  
> -    if ( phy_addr >= 32) {
> -        return OGMA_ERR_RANGE;
> -    }
> -
>      if ( !ctrl_p->param.use_gmac_flag) {
>          return OGMA_ERR_NOTAVAIL;
>      }
> @@ -966,17 +939,17 @@ ogma_err_t ogma_get_phy_link_status (
>      pfdep_memset( phy_link_status_p, 0, sizeof( ogma_phy_link_status_t) );
>  
>      /* Read PHY CONTROL Register */
> -    tmp = ogma_get_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_CONTROL);
> +    tmp = ogma_get_phy_reg_sub( ctrl_p,  OGMA_PHY_REG_ADDR_CONTROL);
>  
>      /* Read PHY STATUS Register */
> -    value = ogma_get_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_STATUS);
> +    value = ogma_get_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_STATUS);
>  
>      /* check latched_link_down_flag */
>      if ( ( value & ( 1U << OGMA_PHY_STATUS_REG_LINK_STATUS) ) == 0) {
>          phy_link_status_p->latched_link_down_flag = OGMA_TRUE;
>  
>          /* Read PHY STATUS Register */
> -        value = ogma_get_phy_reg_sub( ctrl_p, phy_addr, OGMA_PHY_REG_ADDR_STATUS);
> +        value = ogma_get_phy_reg_sub( ctrl_p, OGMA_PHY_REG_ADDR_STATUS);
>  
>      }
>  
> @@ -1036,12 +1009,10 @@ ogma_err_t ogma_get_phy_link_status (
>  
>              /* Read MASTER-SLAVE Control Register */
>              value = ogma_get_phy_reg_sub( ctrl_p,
> -                                          phy_addr,
>                                            OGMA_PHY_REG_ADDR_MASTER_SLAVE_CONTROL);
>  
>              /* Read MASTER-SLAVE Status Register */
>              tmp = ogma_get_phy_reg_sub( ctrl_p,
> -                                        phy_addr,
>                                          OGMA_PHY_REG_ADDR_MASTER_SLAVE_STATUS);
>  
>              /* Check Current Link Speed */
> @@ -1061,12 +1032,10 @@ ogma_err_t ogma_get_phy_link_status (
>  
>                  /* Read Auto-Negotiation Advertisement register */
>                  value = ogma_get_phy_reg_sub( ctrl_p,
> -                                              phy_addr,
>                                                OGMA_PHY_REG_ADDR_AUTO_NEGO_ABILTY);
>  
>                  /* Read Auto-Negotiation Link Partner Base Page Ability register */
>                  tmp = ogma_get_phy_reg_sub( ctrl_p,
> -                                             phy_addr,
>                                               OGMA_PHY_REG_ADDR_AUTO_NEGO_LINK_PATNER_ABILTY);
>  
>                  value = ( ( ( value & tmp) >> OGMA_PHY_ANA_REG_TAF) &
> @@ -1109,13 +1078,11 @@ ogma_err_t ogma_get_phy_link_status (
>  
>          /* Read EEE advertisement register */
>          value = ogma_get_phy_mmd_reg_sub( ctrl_p,
> -                                          phy_addr,
>                                            OGMA_PHY_DEV_ADDR_AUTO_NEGO,
>                                            OGMA_PHY_AUTO_NEGO_REG_ADDR_EEE_ADVERTISE);
>  
>          /* Read EEE link partner ability register */
>          tmp = ogma_get_phy_mmd_reg_sub( ctrl_p,
> -                                        phy_addr,
>                                          OGMA_PHY_DEV_ADDR_AUTO_NEGO,
>                                          OGMA_PHY_AUTO_NEGO_REG_ADDR_EEE_LP_ABILITY);
>  
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_internal.h b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_internal.h
> index ed09a7ada85d..a7bc69cf0777 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_internal.h
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_internal.h
> @@ -111,6 +111,8 @@ struct ogma_ctrl_s{
>  
>      pfdep_phys_addr_t dummy_desc_entry_phys_addr;
>  
> +    ogma_uint8 phy_addr;
> +
>  #ifdef OGMA_CONFIG_REC_STAT
>      /**
>       * Statistics information.
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
> index 4dec66313aa1..7481d2da2d24 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
> @@ -388,6 +388,12 @@ ogma_err_t ogma_init (
>          return OGMA_ERR_DATA;
>      }
>  
> +    if ( param_p->phy_addr >= 32) {
> +        pfdep_print( PFDEP_DEBUG_LEVEL_FATAL,
> +                     "Error: phy_addr out of range\n");
> +        return OGMA_ERR_DATA;
> +    }
> +
>      ogma_err = ogma_probe_hardware( base_addr);
>  
>      if ( ogma_err != OGMA_ERR_OK) {
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-platforms PATCH v1 2/3] NetsecDxe: put phy in loopback mode in order to guarantee stable RXCLK input
  2019-07-22 11:56 ` [edk2-platforms PATCH v1 2/3] NetsecDxe: put phy in loopback mode in order to guarantee stable RXCLK input Masahisa Kojima
@ 2019-07-23 15:49   ` Leif Lindholm
  0 siblings, 0 replies; 8+ messages in thread
From: Leif Lindholm @ 2019-07-23 15:49 UTC (permalink / raw)
  To: Masahisa Kojima; +Cc: devel, ard.biesheuvel, okamoto.satoru

On Mon, Jul 22, 2019 at 08:56:35PM +0900, Masahisa Kojima wrote:
> NETSEC hardware requires stable RXCLK input upon initialization
> triggered with DISCORE = 0.
> However, RXCLK input could be unstable depending on phy chipset
> and deployed network environment, which could cause NETSEC to
> hang up during initialization.
> 
> We solve this platform/environment dependent issue by temporarily
> putting phy in loopback mode, then we can expect the stable RXCLK input.
> 
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> Signed-off-by: Satoru Okamoto <okamoto.satoru@socionext.com>
> ---
>  .../netsec_sdk/src/ogma_misc.c                | 72 ++++++++++++++++++-
>  .../netsec_for_uefi/netsec_sdk/src/ogma_reg.h |  4 ++
>  2 files changed, 75 insertions(+), 1 deletion(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
> index 7481d2da2d24..07308d38a5c2 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c
> @@ -35,7 +35,6 @@ static const ogma_uint32 desc_ring_config_reg_addr[OGMA_DESC_RING_ID_MAX + 1] =
>  
>  };
>  
> -

Please, no unrelated whitespace changes.

>  /* Internal function definition*/
>  #ifndef OGMA_CONFIG_DISABLE_CLK_CTRL
>  STATIC void ogma_set_clk_en_reg (
> @@ -327,6 +326,60 @@ STATIC ogma_uint32 ogma_calc_pkt_ctrl_reg_param (
>      return param;
>  }
>  
> +STATIC
> +void
> +ogma_pre_init_microengine (
> +  ogma_handle_t ogma_handle
> +  )
> +{
> +  UINT16 Data;
> +
> +  /* Remove dormant settings */
> +  Data = ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
> +    ~((1U << OGMA_PHY_CONTROL_REG_POWER_DOWN) |
> +      (1U << OGMA_PHY_CONTROL_REG_ISOLATE));

I am unsure of how much we should try to adhere to TianoCore coding
style in this imported module, but the above does not look correct
regardless. Should not the second/third lines should be aligned
relative to the ogma_get_phy_reg call rather than the variable name...

> +
> +  ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data);
> +
> +  while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
> +          ((1U << OGMA_PHY_CONTROL_REG_POWER_DOWN) |
> +           (1U << OGMA_PHY_CONTROL_REG_ISOLATE))) != 0);

...like it is here.

> +
> +  /* Put phy in loopback mode to guarantee RXCLK input */
> +  Data |= (1U << OGMA_PHY_CONTROL_REG_LOOPBACK);
> +  
> +  ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data);
> +
> +  while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
> +          (1U << OGMA_PHY_CONTROL_REG_LOOPBACK)) == 0);
> +}
> +
> +STATIC
> +void
> +ogma_post_init_microengine (
> +  IN ogma_handle_t ogma_handle
> +  )
> +{
> +  UINT16 Data;
> +
> +  /* Get phy back to normal operation */
> +  Data = ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
> +    ~(1U << OGMA_PHY_CONTROL_REG_LOOPBACK);

Same indentation comment as above.

> +
> +  ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data);
> +
> +  while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
> +          (1U << OGMA_PHY_CONTROL_REG_LOOPBACK)) != 0);
> +
> +  Data |= (1U << OGMA_PHY_CONTROL_REG_RESET);
> +  
> +  /* Apply software reset */
> +  ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data);
> +
> +  while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) &
> +          (1U << OGMA_PHY_CONTROL_REG_RESET)) != 0);
> +}
> +
>  ogma_err_t ogma_init (
>      void *base_addr,
>      pfdep_dev_handle_t dev_handle,
> @@ -551,6 +604,16 @@ ogma_err_t ogma_init (
>      ogma_write_reg( ctrl_p, OGMA_REG_ADDR_DMA_TMR_CTRL,
>                      ( ogma_uint32)( ( OGMA_CONFIG_CLK_HZ / OGMA_CLK_MHZ) - 1) );
>  
> +    /*
> +     * Do pre-initialization tasks for microengine
> +     *
> +     * In particular, we put phy in loopback mode
> +     * in order to make sure RXCLK keeps provided to mac
> +     * irrespective of phy link status,
> +     * which is required for microengine intialization.
> +     */

Good use of comment here, and below.
*But*, if the next thing was not the bit taking the PHY back out of
loopback mode, I would have had to spend time searching for whether
that was done. Could you add a line saying "this will be disabled once
initialization complete"?

/
    Leif

> +    ogma_pre_init_microengine (ctrl_p);
> +
>      /* start microengines */
>      ogma_write_reg( ctrl_p, OGMA_REG_ADDR_DIS_CORE, 0);
>  
> @@ -573,6 +636,13 @@ ogma_err_t ogma_init (
>          goto err;
>      }
>  
> +    /*
> +     * Do post-initialization tasks for microengine
> +     *
> +     * We put phy in normal mode and apply reset.
> +     */
> +    ogma_post_init_microengine (ctrl_p);
> +
>      /* clear microcode load end status */
>      ogma_write_reg( ctrl_p, OGMA_REG_ADDR_TOP_STATUS,
>                      OGMA_TOP_IRQ_REG_ME_START);
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h
> index 30c716352b37..ca769084cb31 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h
> @@ -138,8 +138,12 @@
>  /* bit fields for PHY CONTROL Register */
>  #define OGMA_PHY_CONTROL_REG_SPEED_SELECTION_MSB (6)
>  #define OGMA_PHY_CONTROL_REG_DUPLEX_MODE         (8)
> +#define OGMA_PHY_CONTROL_REG_ISOLATE             (10)
> +#define OGMA_PHY_CONTROL_REG_POWER_DOWN          (11)
>  #define OGMA_PHY_CONTROL_REG_AUTO_NEGO_ENABLE    (12)
>  #define OGMA_PHY_CONTROL_REG_SPEED_SELECTION_LSB (13)
> +#define OGMA_PHY_CONTROL_REG_LOOPBACK            (14)
> +#define OGMA_PHY_CONTROL_REG_RESET               (15)
>  
>  /* bit fields for PHY STATUS Register */
>  #define OGMA_PHY_STATUS_REG_LINK_STATUS       (2)
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-platforms PATCH v1 3/3] NetsecDxe: SnpInitialize() waits for media linking up
  2019-07-22 11:56 ` [edk2-platforms PATCH v1 3/3] NetsecDxe: SnpInitialize() waits for media linking up Masahisa Kojima
@ 2019-07-23 16:14   ` Leif Lindholm
  2019-07-24  6:42     ` Masahisa Kojima
  0 siblings, 1 reply; 8+ messages in thread
From: Leif Lindholm @ 2019-07-23 16:14 UTC (permalink / raw)
  To: Masahisa Kojima; +Cc: devel, ard.biesheuvel, okamoto.satoru

On Mon, Jul 22, 2019 at 08:56:36PM +0900, Masahisa Kojima wrote:
> The latest NetsecDxe requires issueing phy reset at the
> last stage of initialization to safely exit loopback mode.
> However, as a result, it takes a couple of seconds for link state
> to get stable, which could cause auto-chosen pxeboot to fail
> due to MediaPresent check error.
> 
> This patch adds link state check with 5s timeout in NetsecDxe
> initialization. The timeout value can be adjustable via
> configuration file.
> 
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> Signed-off-by: Satoru Okamoto <okamoto.satoru@socionext.com>
> ---
>  .../Socionext/DeveloperBox/DeveloperBox.dsc   |   1 +
>  .../Drivers/Net/NetsecDxe/NetsecDxe.c         | 232 ++++++++----------
>  .../Drivers/Net/NetsecDxe/NetsecDxe.dec       |   1 +
>  .../Drivers/Net/NetsecDxe/NetsecDxe.inf       |   1 +
>  4 files changed, 110 insertions(+), 125 deletions(-)

Please run edk2/BaseTools/Scripts/SetupGit.py from within this
repository. It sets up some common options that help with reviewing
patches.

(--stat=1000 is one option that can unfortunately not be overridden in
this way)

> 
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> index 97fb8c410c60..af149607f3ce 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> @@ -138,6 +138,7 @@
>    gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStartThreshold|36
>    gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStopThreshold|48
>    gNetsecDxeTokenSpaceGuid.PcdPauseTime|256
> +  gNetsecDxeTokenSpaceGuid.PcdMediaDetectTimeoutOnBoot|5

Please insert alphabetically sorted.

>  
>    gSynQuacerTokenSpaceGuid.PcdNetsecEepromBase|0x08080000
>    gSynQuacerTokenSpaceGuid.PcdNetsecPhyAddress|7
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> index 0b91d4af44a3..a304e02208fa 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> @@ -169,6 +169,98 @@ ExitUnlock:
>    return Status;
>  }
>  
> +EFI_STATUS
> +EFIAPI
> +NetsecUpdateLink (
> +  IN  EFI_SIMPLE_NETWORK_PROTOCOL    *Snp
> +  )
> +{
> +  NETSEC_DRIVER                 *LanDriver;
> +  ogma_phy_link_status_t        phy_link_status;
> +  ogma_gmac_mode_t              ogma_gmac_mode;
> +  ogma_err_t                    ogma_err;
> +  BOOLEAN                       ValidFlag;
> +  ogma_gmac_mode_t              GmacMode;
> +  BOOLEAN                       RxRunningFlag;
> +  BOOLEAN                       TxRunningFlag;
> +  EFI_STATUS                    ErrorStatus;
> +
> +  LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
> +
> +  // Update the media status
> +  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> +               &phy_link_status);
> +  if (ogma_err != OGMA_ERR_OK) {
> +    DEBUG ((DEBUG_ERROR,
> +      "NETSEC: ogma_get_phy_link_status failed with error code: %d\n",
> +      (INT32)ogma_err));
> +    ErrorStatus = EFI_DEVICE_ERROR;
> +    goto Fail;
> +  }
> +
> +  // Update the GMAC status
> +  ogma_err = ogma_get_gmac_status (LanDriver->Handle, &ValidFlag, &GmacMode,
> +               &RxRunningFlag, &TxRunningFlag);
> +  if (ogma_err != OGMA_ERR_OK) {
> +    DEBUG ((DEBUG_ERROR,
> +      "NETSEC: ogma_get_gmac_status failed with error code: %d\n",
> +      (INT32)ogma_err));
> +    ErrorStatus = EFI_DEVICE_ERROR;
> +    goto Fail;
> +  }
> +
> +  // Stop GMAC when GMAC is running and physical link is down
> +  if (RxRunningFlag && TxRunningFlag && !phy_link_status.up_flag) {
> +    ogma_err = ogma_stop_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> +    if (ogma_err != OGMA_ERR_OK) {
> +      DEBUG ((DEBUG_ERROR,
> +        "NETSEC: ogma_stop_gmac() failed with error status %d\n",
> +        ogma_err));
> +      ErrorStatus = EFI_DEVICE_ERROR;
> +      goto Fail;
> +    }
> +  }
> +
> +  // Start GMAC when GMAC is stopped and physical link is up
> +  if (!RxRunningFlag && !TxRunningFlag && phy_link_status.up_flag) {
> +    ZeroMem (&ogma_gmac_mode, sizeof (ogma_gmac_mode_t));
> +    ogma_gmac_mode.link_speed = phy_link_status.link_speed;
> +    ogma_gmac_mode.half_duplex_flag = (ogma_bool)phy_link_status.half_duplex_flag;
> +    if (!phy_link_status.half_duplex_flag && FixedPcdGet8 (PcdFlowCtrl)) {
> +      ogma_gmac_mode.flow_ctrl_enable_flag     = FixedPcdGet8 (PcdFlowCtrl);
> +      ogma_gmac_mode.flow_ctrl_start_threshold = FixedPcdGet16 (PcdFlowCtrlStartThreshold);
> +      ogma_gmac_mode.flow_ctrl_stop_threshold  = FixedPcdGet16 (PcdFlowCtrlStopThreshold);
> +      ogma_gmac_mode.pause_time                = FixedPcdGet16 (PcdPauseTime);
> +    }
> +
> +    ogma_err = ogma_set_gmac_mode (LanDriver->Handle, &ogma_gmac_mode);
> +    if (ogma_err != OGMA_ERR_OK) {
> +      DEBUG ((DEBUG_ERROR,
> +        "NETSEC: ogma_set_gmac() failed with error status %d\n",
> +        (INT32)ogma_err));
> +      ErrorStatus = EFI_DEVICE_ERROR;
> +      goto Fail;
> +    }
> +
> +    ogma_err = ogma_start_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> +    if (ogma_err != OGMA_ERR_OK) {
> +      DEBUG ((DEBUG_ERROR,
> +        "NETSEC: ogma_start_gmac() failed with error status %d\n",
> +        (INT32)ogma_err));
> +      ErrorStatus = EFI_DEVICE_ERROR;
> +      goto Fail;
> +    }
> +  }
> +
> +  /* Updating link status for external guery */
> +  Snp->Mode->MediaPresent = phy_link_status.up_flag;
> +  return EFI_SUCCESS;
> +
> +Fail:
> +  Snp->Mode->MediaPresent = FALSE;
> +  return ErrorStatus;
> +}
> +
>  /*
>   *  UEFI Initialize() function
>   */
> @@ -185,9 +277,9 @@ SnpInitialize (
>    EFI_TPL                 SavedTpl;
>    EFI_STATUS              Status;
>  
> -  ogma_phy_link_status_t  phy_link_status;
>    ogma_err_t              ogma_err;
> -  ogma_gmac_mode_t        ogma_gmac_mode;
> +
> +  UINT32                  Index;
>  
>    // Check Snp Instance
>    if (Snp == NULL) {
> @@ -271,48 +363,18 @@ SnpInitialize (
>    ogma_disable_desc_ring_irq (LanDriver->Handle, OGMA_DESC_RING_ID_NRM_TX,
>                                OGMA_CH_IRQ_REG_EMPTY);
>  
> -  // Stop and restart the physical link
> -  ogma_err = ogma_stop_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> -  if (ogma_err != OGMA_ERR_OK) {
> -    DEBUG ((DEBUG_ERROR,
> -      "NETSEC: ogma_stop_gmac() failed with error status %d\n",
> -      ogma_err));
> -    ReturnUnlock (EFI_DEVICE_ERROR);
> -  }
> -
> -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> -               &phy_link_status);
> -  if (ogma_err != OGMA_ERR_OK) {
> -    DEBUG ((DEBUG_ERROR,
> -      "NETSEC: ogma_get_phy_link_status() failed error code %d\n",
> -      (INT32)ogma_err));
> -    ReturnUnlock (EFI_DEVICE_ERROR);
> -  }
> -
> -  SetMem (&ogma_gmac_mode, sizeof (ogma_gmac_mode_t), 0);
> -  ogma_gmac_mode.link_speed = phy_link_status.link_speed;
> -  ogma_gmac_mode.half_duplex_flag = (ogma_bool)phy_link_status.half_duplex_flag;
> -  if ((!phy_link_status.half_duplex_flag) && FixedPcdGet8 (PcdFlowCtrl)) {
> -    ogma_gmac_mode.flow_ctrl_enable_flag     = FixedPcdGet8 (PcdFlowCtrl);
> -    ogma_gmac_mode.flow_ctrl_start_threshold = FixedPcdGet16 (PcdFlowCtrlStartThreshold);
> -    ogma_gmac_mode.flow_ctrl_stop_threshold  = FixedPcdGet16 (PcdFlowCtrlStopThreshold);
> -    ogma_gmac_mode.pause_time                = FixedPcdGet16 (PcdPauseTime);
> -  }
> -
> -  ogma_err = ogma_set_gmac_mode (LanDriver->Handle, &ogma_gmac_mode);
> -  if (ogma_err != OGMA_ERR_OK) {
> -    DEBUG ((DEBUG_ERROR,
> -      "NETSEC: ogma_set_gmac() failed with error status %d\n",
> -      (INT32)ogma_err));
> -    ReturnUnlock (EFI_DEVICE_ERROR);
> -  }
> -
> -  ogma_err = ogma_start_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> -  if (ogma_err != OGMA_ERR_OK) {
> -    DEBUG ((DEBUG_ERROR,
> -      "NETSEC: ogma_start_gmac() failed with error status %d\n",
> -      (INT32)ogma_err));
> -    ReturnUnlock (EFI_DEVICE_ERROR);
> +  // Wait for media linking up
> +  for (Index = 0; Index < (UINT32)FixedPcdGet8 (PcdMediaDetectTimeoutOnBoot) * 10; Index++) {
> +    Status = NetsecUpdateLink (Snp);
> +    if (Status != EFI_SUCCESS) {
> +      ReturnUnlock (EFI_DEVICE_ERROR);
> +    }
> +
> +    if (Snp->Mode->MediaPresent) {
> +      break;
> +    }
> +
> +    MicroSecondDelay(100000);
>    }
>  
>    // Declare the driver as initialized
> @@ -420,14 +482,6 @@ NetsecPollPhyStatus (
>    )
>  {
>    EFI_SIMPLE_NETWORK_PROTOCOL   *Snp;
> -  NETSEC_DRIVER                 *LanDriver;
> -  ogma_phy_link_status_t        phy_link_status;
> -  ogma_gmac_mode_t              ogma_gmac_mode;
> -  ogma_err_t                    ogma_err;
> -  BOOLEAN                       ValidFlag;
> -  ogma_gmac_mode_t              GmacMode;
> -  BOOLEAN                       RxRunningFlag;
> -  BOOLEAN                       TxRunningFlag;
>  
>    Snp = (EFI_SIMPLE_NETWORK_PROTOCOL *)Context;
>    if (Snp == NULL) {
> @@ -435,66 +489,7 @@ NetsecPollPhyStatus (
>      return;
>    }
>  
> -  LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
> -
> -  // Update the media status
> -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> -               &phy_link_status);
> -  if (ogma_err != OGMA_ERR_OK) {
> -    DEBUG ((DEBUG_ERROR,
> -      "NETSEC: ogma_get_phy_link_status failed with error code: %d\n",
> -      (INT32)ogma_err));
> -    return;
> -  }
> -
> -  // Update the GMAC status
> -  ogma_err = ogma_get_gmac_status (LanDriver->Handle, &ValidFlag, &GmacMode,
> -               &RxRunningFlag, &TxRunningFlag);
> -  if (ogma_err != OGMA_ERR_OK) {
> -    DEBUG ((DEBUG_ERROR,
> -      "NETSEC: ogma_get_gmac_status failed with error code: %d\n",
> -      (INT32)ogma_err));
> -    return;
> -  }
> -
> -  // Stop GMAC when GMAC is running and physical link is down
> -  if (RxRunningFlag && TxRunningFlag && !phy_link_status.up_flag) {
> -    ogma_err = ogma_stop_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> -    if (ogma_err != OGMA_ERR_OK) {
> -      DEBUG ((DEBUG_ERROR,
> -        "NETSEC: ogma_stop_gmac() failed with error status %d\n",
> -        ogma_err));
> -      return;
> -    }
> -  }
> -
> -  // Start GMAC when GMAC is stopped and physical link is up
> -  if (!RxRunningFlag && !TxRunningFlag && phy_link_status.up_flag) {
> -    ZeroMem (&ogma_gmac_mode, sizeof (ogma_gmac_mode_t));
> -    ogma_gmac_mode.link_speed = phy_link_status.link_speed;
> -    ogma_gmac_mode.half_duplex_flag = (ogma_bool)phy_link_status.half_duplex_flag;
> -    if (!phy_link_status.half_duplex_flag && FixedPcdGet8 (PcdFlowCtrl)) {
> -      ogma_gmac_mode.flow_ctrl_enable_flag     = FixedPcdGet8 (PcdFlowCtrl);
> -      ogma_gmac_mode.flow_ctrl_start_threshold = FixedPcdGet16 (PcdFlowCtrlStartThreshold);
> -      ogma_gmac_mode.flow_ctrl_stop_threshold  = FixedPcdGet16 (PcdFlowCtrlStopThreshold);
> -      ogma_gmac_mode.pause_time                = FixedPcdGet16 (PcdPauseTime);
> -    }
> -
> -    ogma_err = ogma_set_gmac_mode (LanDriver->Handle, &ogma_gmac_mode);
> -    if (ogma_err != OGMA_ERR_OK) {
> -      DEBUG ((DEBUG_ERROR,
> -        "NETSEC: ogma_set_gmac() failed with error status %d\n",
> -        (INT32)ogma_err));
> -      return;
> -    }
> -
> -    ogma_err = ogma_start_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> -    if (ogma_err != OGMA_ERR_OK) {
> -      DEBUG ((DEBUG_ERROR,
> -        "NETSEC: ogma_start_gmac() failed with error status %d\n",
> -        (INT32)ogma_err));
> -    }
> -  }
> +  NetsecUpdateLink (Snp);
>  }
>  
>  /*
> @@ -631,7 +626,6 @@ SnpGetStatus (
>    pfdep_pkt_handle_t        pkt_handle;
>    LIST_ENTRY                *Link;
>  
> -  ogma_phy_link_status_t  phy_link_status;
>    ogma_err_t              ogma_err;
>  
>    // Check preliminaries
> @@ -661,18 +655,6 @@ SnpGetStatus (
>    // Find the LanDriver structure
>    LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
>  
> -  // Update the media status
> -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> -               &phy_link_status);
> -  if (ogma_err != OGMA_ERR_OK) {
> -    DEBUG ((DEBUG_ERROR,
> -      "NETSEC: ogma_get_phy_link_status failed with error code: %d\n",
> -      (INT32)ogma_err));
> -    ReturnUnlock (EFI_DEVICE_ERROR);
> -  }
> -
> -  Snp->Mode->MediaPresent = phy_link_status.up_flag;
> -
>    ogma_err = ogma_clean_tx_desc_ring (LanDriver->Handle,
>                                        OGMA_DESC_RING_ID_NRM_TX);
>  
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec
> index 6b9f60293879..016eba5ef695 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec
> @@ -38,3 +38,4 @@
>    gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStartThreshold|0x0|UINT16|0x00000006
>    gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStopThreshold|0x0|UINT16|0x00000007
>    gNetsecDxeTokenSpaceGuid.PcdPauseTime|0x0|UINT16|0x00000008
> +  gNetsecDxeTokenSpaceGuid.PcdMediaDetectTimeoutOnBoot|0x0|UINT8|0x00000009

Please insert alphabetically sorted.

> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf
> index 49dd28efc65b..818014e6d257 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf
> @@ -62,3 +62,4 @@
>    gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStopThreshold
>    gNetsecDxeTokenSpaceGuid.PcdJumboPacket
>    gNetsecDxeTokenSpaceGuid.PcdPauseTime
> +  gNetsecDxeTokenSpaceGuid.PcdMediaDetectTimeoutOnBoot

Please insert slphabetcally sorted.

/
    Leif

> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-platforms PATCH v1 3/3] NetsecDxe: SnpInitialize() waits for media linking up
  2019-07-23 16:14   ` Leif Lindholm
@ 2019-07-24  6:42     ` Masahisa Kojima
  0 siblings, 0 replies; 8+ messages in thread
From: Masahisa Kojima @ 2019-07-24  6:42 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel-groups-io, Ard Biesheuvel, Satoru Okamoto

On Wed, 24 Jul 2019 at 01:14, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Mon, Jul 22, 2019 at 08:56:36PM +0900, Masahisa Kojima wrote:
> > The latest NetsecDxe requires issueing phy reset at the
> > last stage of initialization to safely exit loopback mode.
> > However, as a result, it takes a couple of seconds for link state
> > to get stable, which could cause auto-chosen pxeboot to fail
> > due to MediaPresent check error.
> >
> > This patch adds link state check with 5s timeout in NetsecDxe
> > initialization. The timeout value can be adjustable via
> > configuration file.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > Signed-off-by: Satoru Okamoto <okamoto.satoru@socionext.com>
> > ---
> >  .../Socionext/DeveloperBox/DeveloperBox.dsc   |   1 +
> >  .../Drivers/Net/NetsecDxe/NetsecDxe.c         | 232 ++++++++----------
> >  .../Drivers/Net/NetsecDxe/NetsecDxe.dec       |   1 +
> >  .../Drivers/Net/NetsecDxe/NetsecDxe.inf       |   1 +
> >  4 files changed, 110 insertions(+), 125 deletions(-)
>
> Please run edk2/BaseTools/Scripts/SetupGit.py from within this
> repository. It sets up some common options that help with reviewing
> patches.
>
> (--stat=1000 is one option that can unfortunately not be overridden in
> this way)

I understand.
I address all your comments, then send v2 patch.

> >
> > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > index 97fb8c410c60..af149607f3ce 100644
> > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > @@ -138,6 +138,7 @@
> >    gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStartThreshold|36
> >    gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStopThreshold|48
> >    gNetsecDxeTokenSpaceGuid.PcdPauseTime|256
> > +  gNetsecDxeTokenSpaceGuid.PcdMediaDetectTimeoutOnBoot|5
>
> Please insert alphabetically sorted.
>
> >
> >    gSynQuacerTokenSpaceGuid.PcdNetsecEepromBase|0x08080000
> >    gSynQuacerTokenSpaceGuid.PcdNetsecPhyAddress|7
> > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> > index 0b91d4af44a3..a304e02208fa 100644
> > --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> > +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.c
> > @@ -169,6 +169,98 @@ ExitUnlock:
> >    return Status;
> >  }
> >
> > +EFI_STATUS
> > +EFIAPI
> > +NetsecUpdateLink (
> > +  IN  EFI_SIMPLE_NETWORK_PROTOCOL    *Snp
> > +  )
> > +{
> > +  NETSEC_DRIVER                 *LanDriver;
> > +  ogma_phy_link_status_t        phy_link_status;
> > +  ogma_gmac_mode_t              ogma_gmac_mode;
> > +  ogma_err_t                    ogma_err;
> > +  BOOLEAN                       ValidFlag;
> > +  ogma_gmac_mode_t              GmacMode;
> > +  BOOLEAN                       RxRunningFlag;
> > +  BOOLEAN                       TxRunningFlag;
> > +  EFI_STATUS                    ErrorStatus;
> > +
> > +  LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
> > +
> > +  // Update the media status
> > +  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> > +               &phy_link_status);
> > +  if (ogma_err != OGMA_ERR_OK) {
> > +    DEBUG ((DEBUG_ERROR,
> > +      "NETSEC: ogma_get_phy_link_status failed with error code: %d\n",
> > +      (INT32)ogma_err));
> > +    ErrorStatus = EFI_DEVICE_ERROR;
> > +    goto Fail;
> > +  }
> > +
> > +  // Update the GMAC status
> > +  ogma_err = ogma_get_gmac_status (LanDriver->Handle, &ValidFlag, &GmacMode,
> > +               &RxRunningFlag, &TxRunningFlag);
> > +  if (ogma_err != OGMA_ERR_OK) {
> > +    DEBUG ((DEBUG_ERROR,
> > +      "NETSEC: ogma_get_gmac_status failed with error code: %d\n",
> > +      (INT32)ogma_err));
> > +    ErrorStatus = EFI_DEVICE_ERROR;
> > +    goto Fail;
> > +  }
> > +
> > +  // Stop GMAC when GMAC is running and physical link is down
> > +  if (RxRunningFlag && TxRunningFlag && !phy_link_status.up_flag) {
> > +    ogma_err = ogma_stop_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> > +    if (ogma_err != OGMA_ERR_OK) {
> > +      DEBUG ((DEBUG_ERROR,
> > +        "NETSEC: ogma_stop_gmac() failed with error status %d\n",
> > +        ogma_err));
> > +      ErrorStatus = EFI_DEVICE_ERROR;
> > +      goto Fail;
> > +    }
> > +  }
> > +
> > +  // Start GMAC when GMAC is stopped and physical link is up
> > +  if (!RxRunningFlag && !TxRunningFlag && phy_link_status.up_flag) {
> > +    ZeroMem (&ogma_gmac_mode, sizeof (ogma_gmac_mode_t));
> > +    ogma_gmac_mode.link_speed = phy_link_status.link_speed;
> > +    ogma_gmac_mode.half_duplex_flag = (ogma_bool)phy_link_status.half_duplex_flag;
> > +    if (!phy_link_status.half_duplex_flag && FixedPcdGet8 (PcdFlowCtrl)) {
> > +      ogma_gmac_mode.flow_ctrl_enable_flag     = FixedPcdGet8 (PcdFlowCtrl);
> > +      ogma_gmac_mode.flow_ctrl_start_threshold = FixedPcdGet16 (PcdFlowCtrlStartThreshold);
> > +      ogma_gmac_mode.flow_ctrl_stop_threshold  = FixedPcdGet16 (PcdFlowCtrlStopThreshold);
> > +      ogma_gmac_mode.pause_time                = FixedPcdGet16 (PcdPauseTime);
> > +    }
> > +
> > +    ogma_err = ogma_set_gmac_mode (LanDriver->Handle, &ogma_gmac_mode);
> > +    if (ogma_err != OGMA_ERR_OK) {
> > +      DEBUG ((DEBUG_ERROR,
> > +        "NETSEC: ogma_set_gmac() failed with error status %d\n",
> > +        (INT32)ogma_err));
> > +      ErrorStatus = EFI_DEVICE_ERROR;
> > +      goto Fail;
> > +    }
> > +
> > +    ogma_err = ogma_start_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> > +    if (ogma_err != OGMA_ERR_OK) {
> > +      DEBUG ((DEBUG_ERROR,
> > +        "NETSEC: ogma_start_gmac() failed with error status %d\n",
> > +        (INT32)ogma_err));
> > +      ErrorStatus = EFI_DEVICE_ERROR;
> > +      goto Fail;
> > +    }
> > +  }
> > +
> > +  /* Updating link status for external guery */
> > +  Snp->Mode->MediaPresent = phy_link_status.up_flag;
> > +  return EFI_SUCCESS;
> > +
> > +Fail:
> > +  Snp->Mode->MediaPresent = FALSE;
> > +  return ErrorStatus;
> > +}
> > +
> >  /*
> >   *  UEFI Initialize() function
> >   */
> > @@ -185,9 +277,9 @@ SnpInitialize (
> >    EFI_TPL                 SavedTpl;
> >    EFI_STATUS              Status;
> >
> > -  ogma_phy_link_status_t  phy_link_status;
> >    ogma_err_t              ogma_err;
> > -  ogma_gmac_mode_t        ogma_gmac_mode;
> > +
> > +  UINT32                  Index;
> >
> >    // Check Snp Instance
> >    if (Snp == NULL) {
> > @@ -271,48 +363,18 @@ SnpInitialize (
> >    ogma_disable_desc_ring_irq (LanDriver->Handle, OGMA_DESC_RING_ID_NRM_TX,
> >                                OGMA_CH_IRQ_REG_EMPTY);
> >
> > -  // Stop and restart the physical link
> > -  ogma_err = ogma_stop_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> > -  if (ogma_err != OGMA_ERR_OK) {
> > -    DEBUG ((DEBUG_ERROR,
> > -      "NETSEC: ogma_stop_gmac() failed with error status %d\n",
> > -      ogma_err));
> > -    ReturnUnlock (EFI_DEVICE_ERROR);
> > -  }
> > -
> > -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> > -               &phy_link_status);
> > -  if (ogma_err != OGMA_ERR_OK) {
> > -    DEBUG ((DEBUG_ERROR,
> > -      "NETSEC: ogma_get_phy_link_status() failed error code %d\n",
> > -      (INT32)ogma_err));
> > -    ReturnUnlock (EFI_DEVICE_ERROR);
> > -  }
> > -
> > -  SetMem (&ogma_gmac_mode, sizeof (ogma_gmac_mode_t), 0);
> > -  ogma_gmac_mode.link_speed = phy_link_status.link_speed;
> > -  ogma_gmac_mode.half_duplex_flag = (ogma_bool)phy_link_status.half_duplex_flag;
> > -  if ((!phy_link_status.half_duplex_flag) && FixedPcdGet8 (PcdFlowCtrl)) {
> > -    ogma_gmac_mode.flow_ctrl_enable_flag     = FixedPcdGet8 (PcdFlowCtrl);
> > -    ogma_gmac_mode.flow_ctrl_start_threshold = FixedPcdGet16 (PcdFlowCtrlStartThreshold);
> > -    ogma_gmac_mode.flow_ctrl_stop_threshold  = FixedPcdGet16 (PcdFlowCtrlStopThreshold);
> > -    ogma_gmac_mode.pause_time                = FixedPcdGet16 (PcdPauseTime);
> > -  }
> > -
> > -  ogma_err = ogma_set_gmac_mode (LanDriver->Handle, &ogma_gmac_mode);
> > -  if (ogma_err != OGMA_ERR_OK) {
> > -    DEBUG ((DEBUG_ERROR,
> > -      "NETSEC: ogma_set_gmac() failed with error status %d\n",
> > -      (INT32)ogma_err));
> > -    ReturnUnlock (EFI_DEVICE_ERROR);
> > -  }
> > -
> > -  ogma_err = ogma_start_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> > -  if (ogma_err != OGMA_ERR_OK) {
> > -    DEBUG ((DEBUG_ERROR,
> > -      "NETSEC: ogma_start_gmac() failed with error status %d\n",
> > -      (INT32)ogma_err));
> > -    ReturnUnlock (EFI_DEVICE_ERROR);
> > +  // Wait for media linking up
> > +  for (Index = 0; Index < (UINT32)FixedPcdGet8 (PcdMediaDetectTimeoutOnBoot) * 10; Index++) {
> > +    Status = NetsecUpdateLink (Snp);
> > +    if (Status != EFI_SUCCESS) {
> > +      ReturnUnlock (EFI_DEVICE_ERROR);
> > +    }
> > +
> > +    if (Snp->Mode->MediaPresent) {
> > +      break;
> > +    }
> > +
> > +    MicroSecondDelay(100000);
> >    }
> >
> >    // Declare the driver as initialized
> > @@ -420,14 +482,6 @@ NetsecPollPhyStatus (
> >    )
> >  {
> >    EFI_SIMPLE_NETWORK_PROTOCOL   *Snp;
> > -  NETSEC_DRIVER                 *LanDriver;
> > -  ogma_phy_link_status_t        phy_link_status;
> > -  ogma_gmac_mode_t              ogma_gmac_mode;
> > -  ogma_err_t                    ogma_err;
> > -  BOOLEAN                       ValidFlag;
> > -  ogma_gmac_mode_t              GmacMode;
> > -  BOOLEAN                       RxRunningFlag;
> > -  BOOLEAN                       TxRunningFlag;
> >
> >    Snp = (EFI_SIMPLE_NETWORK_PROTOCOL *)Context;
> >    if (Snp == NULL) {
> > @@ -435,66 +489,7 @@ NetsecPollPhyStatus (
> >      return;
> >    }
> >
> > -  LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
> > -
> > -  // Update the media status
> > -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> > -               &phy_link_status);
> > -  if (ogma_err != OGMA_ERR_OK) {
> > -    DEBUG ((DEBUG_ERROR,
> > -      "NETSEC: ogma_get_phy_link_status failed with error code: %d\n",
> > -      (INT32)ogma_err));
> > -    return;
> > -  }
> > -
> > -  // Update the GMAC status
> > -  ogma_err = ogma_get_gmac_status (LanDriver->Handle, &ValidFlag, &GmacMode,
> > -               &RxRunningFlag, &TxRunningFlag);
> > -  if (ogma_err != OGMA_ERR_OK) {
> > -    DEBUG ((DEBUG_ERROR,
> > -      "NETSEC: ogma_get_gmac_status failed with error code: %d\n",
> > -      (INT32)ogma_err));
> > -    return;
> > -  }
> > -
> > -  // Stop GMAC when GMAC is running and physical link is down
> > -  if (RxRunningFlag && TxRunningFlag && !phy_link_status.up_flag) {
> > -    ogma_err = ogma_stop_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> > -    if (ogma_err != OGMA_ERR_OK) {
> > -      DEBUG ((DEBUG_ERROR,
> > -        "NETSEC: ogma_stop_gmac() failed with error status %d\n",
> > -        ogma_err));
> > -      return;
> > -    }
> > -  }
> > -
> > -  // Start GMAC when GMAC is stopped and physical link is up
> > -  if (!RxRunningFlag && !TxRunningFlag && phy_link_status.up_flag) {
> > -    ZeroMem (&ogma_gmac_mode, sizeof (ogma_gmac_mode_t));
> > -    ogma_gmac_mode.link_speed = phy_link_status.link_speed;
> > -    ogma_gmac_mode.half_duplex_flag = (ogma_bool)phy_link_status.half_duplex_flag;
> > -    if (!phy_link_status.half_duplex_flag && FixedPcdGet8 (PcdFlowCtrl)) {
> > -      ogma_gmac_mode.flow_ctrl_enable_flag     = FixedPcdGet8 (PcdFlowCtrl);
> > -      ogma_gmac_mode.flow_ctrl_start_threshold = FixedPcdGet16 (PcdFlowCtrlStartThreshold);
> > -      ogma_gmac_mode.flow_ctrl_stop_threshold  = FixedPcdGet16 (PcdFlowCtrlStopThreshold);
> > -      ogma_gmac_mode.pause_time                = FixedPcdGet16 (PcdPauseTime);
> > -    }
> > -
> > -    ogma_err = ogma_set_gmac_mode (LanDriver->Handle, &ogma_gmac_mode);
> > -    if (ogma_err != OGMA_ERR_OK) {
> > -      DEBUG ((DEBUG_ERROR,
> > -        "NETSEC: ogma_set_gmac() failed with error status %d\n",
> > -        (INT32)ogma_err));
> > -      return;
> > -    }
> > -
> > -    ogma_err = ogma_start_gmac (LanDriver->Handle, OGMA_TRUE, OGMA_TRUE);
> > -    if (ogma_err != OGMA_ERR_OK) {
> > -      DEBUG ((DEBUG_ERROR,
> > -        "NETSEC: ogma_start_gmac() failed with error status %d\n",
> > -        (INT32)ogma_err));
> > -    }
> > -  }
> > +  NetsecUpdateLink (Snp);
> >  }
> >
> >  /*
> > @@ -631,7 +626,6 @@ SnpGetStatus (
> >    pfdep_pkt_handle_t        pkt_handle;
> >    LIST_ENTRY                *Link;
> >
> > -  ogma_phy_link_status_t  phy_link_status;
> >    ogma_err_t              ogma_err;
> >
> >    // Check preliminaries
> > @@ -661,18 +655,6 @@ SnpGetStatus (
> >    // Find the LanDriver structure
> >    LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
> >
> > -  // Update the media status
> > -  ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> > -               &phy_link_status);
> > -  if (ogma_err != OGMA_ERR_OK) {
> > -    DEBUG ((DEBUG_ERROR,
> > -      "NETSEC: ogma_get_phy_link_status failed with error code: %d\n",
> > -      (INT32)ogma_err));
> > -    ReturnUnlock (EFI_DEVICE_ERROR);
> > -  }
> > -
> > -  Snp->Mode->MediaPresent = phy_link_status.up_flag;
> > -
> >    ogma_err = ogma_clean_tx_desc_ring (LanDriver->Handle,
> >                                        OGMA_DESC_RING_ID_NRM_TX);
> >
> > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec
> > index 6b9f60293879..016eba5ef695 100644
> > --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec
> > +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec
> > @@ -38,3 +38,4 @@
> >    gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStartThreshold|0x0|UINT16|0x00000006
> >    gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStopThreshold|0x0|UINT16|0x00000007
> >    gNetsecDxeTokenSpaceGuid.PcdPauseTime|0x0|UINT16|0x00000008
> > +  gNetsecDxeTokenSpaceGuid.PcdMediaDetectTimeoutOnBoot|0x0|UINT8|0x00000009
>
> Please insert alphabetically sorted.
>
> > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf
> > index 49dd28efc65b..818014e6d257 100644
> > --- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf
> > +++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf
> > @@ -62,3 +62,4 @@
> >    gNetsecDxeTokenSpaceGuid.PcdFlowCtrlStopThreshold
> >    gNetsecDxeTokenSpaceGuid.PcdJumboPacket
> >    gNetsecDxeTokenSpaceGuid.PcdPauseTime
> > +  gNetsecDxeTokenSpaceGuid.PcdMediaDetectTimeoutOnBoot
>
> Please insert slphabetcally sorted.
>
> /
>     Leif
>
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-07-24  6:43 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-22 11:56 [edk2-platforms PATCH v1 0/3] Robust Netsec Initialiation Masahisa Kojima
2019-07-22 11:56 ` [edk2-platforms PATCH v1 1/3] NetsecDxe: embed phy address into NETSEC SDK internal structure Masahisa Kojima
2019-07-23 15:41   ` Leif Lindholm
2019-07-22 11:56 ` [edk2-platforms PATCH v1 2/3] NetsecDxe: put phy in loopback mode in order to guarantee stable RXCLK input Masahisa Kojima
2019-07-23 15:49   ` Leif Lindholm
2019-07-22 11:56 ` [edk2-platforms PATCH v1 3/3] NetsecDxe: SnpInitialize() waits for media linking up Masahisa Kojima
2019-07-23 16:14   ` Leif Lindholm
2019-07-24  6:42     ` Masahisa Kojima

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox