From: "Leif Lindholm" <leif.lindholm@linaro.org>
To: Masahisa Kojima <masahisa.kojima@linaro.org>
Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org,
okamoto.satoru@socionext.com
Subject: Re: [edk2-platforms PATCH v1 3/3] NetsecDxe: SnpInitialize() waits for media linking up
Date: Tue, 23 Jul 2019 17:14:05 +0100 [thread overview]
Message-ID: <20190723161405.GL11541@bivouac.eciton.net> (raw)
In-Reply-To: <20190722115636.3413-4-masahisa.kojima@linaro.org>
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
>
next prev parent reply other threads:[~2019-07-23 16:14 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2019-07-24 6:42 ` Masahisa Kojima
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190723161405.GL11541@bivouac.eciton.net \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox