From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=OwYHQhYH; spf=pass (domain: linaro.org, ip: 209.85.128.65, mailfrom: leif.lindholm@linaro.org) Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by groups.io with SMTP; Tue, 23 Jul 2019 09:14:09 -0700 Received: by mail-wm1-f65.google.com with SMTP id l2so39013302wmg.0 for ; Tue, 23 Jul 2019 09:14:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=ZOfmCdzLH4AtqPHWz1cDlKZmKC8WxseeMxuS6Bw8m4w=; b=OwYHQhYHrFrEGpqpeA5jV89stQWjiAFZBdJv5MKNqeEIkt40+Na5FlFhHEbW5qkd9u NPLZlJwwk/U4BL+nd8wPbVMBd9+OrTBhb0+mAu8n6iFoWXmcGI5ELOiw95uNkhG2JM1D 23J4NhSfttQVrN2coUUPP7s4kV2ks9r1DoMeiadS+6YgKIZ+oF5U/u1azB8IpvdtiSWt maBA4roVJRfu4SvsY3w2Zmk41jBRjbgCVG8REe/kmJJy1sKfyvpWLYPk+M2ilykRhRkz MzJHMPUbeM2KoTyaCeqQt1gNTWhcZjjUZVGoytssfr3FPHZaVtGqQU3tEFrhHUs8n5sF xeDQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ZOfmCdzLH4AtqPHWz1cDlKZmKC8WxseeMxuS6Bw8m4w=; b=COHOSTPaR/y6tPcysdwfTWvFurXd2RxWJmF7pmc5fviHcgAf1u0QFb0SA0ITMLCEtv ooZnAgXylu08LmE8qdpG3GSRI5/QRLMIHYOeP7dfZ4anyTj7r8M4ATwf7BnyRVbhGqK+ vlb8LXbKSUqzUixaU9IUufMqf25jZ513C4Bn5YFLpvad0iK7W0AJxYSR4DUVYKs6Sw57 svaxN/oPu6ciKEWh1gYLs7Ygj9TF8nEIpuM2qQCPXatCDE4prkiG2UK1bG6pxsYqyVhn dmiUFwdfFzcAuEA+k1Rwwj66GtMletyDLuCCqOfdik7FjWs8LjIIn3u4pC2OujI9ogXi JZhg== X-Gm-Message-State: APjAAAW7t/7JxJqSYpHHHvZgvKOS0/trBQX+jH1M04U2i4GBnhxcaB9P 7iWSeln9jdHBWt+TWGwNoXeVuQ== X-Google-Smtp-Source: APXvYqxQ3UqaDAtCAqNJ0HUYNpmDKJkzuzMy4HZ7wZQdD9lXUJMdNmZBQgDL5zT37cHI0avmAMJ1Dw== X-Received: by 2002:a1c:f116:: with SMTP id p22mr68284264wmh.70.1563898447321; Tue, 23 Jul 2019 09:14:07 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id v204sm44128657wma.20.2019.07.23.09.14.06 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Tue, 23 Jul 2019 09:14:06 -0700 (PDT) Date: Tue, 23 Jul 2019 17:14:05 +0100 From: "Leif Lindholm" To: Masahisa Kojima 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 Message-ID: <20190723161405.GL11541@bivouac.eciton.net> References: <20190722115636.3413-1-masahisa.kojima@linaro.org> <20190722115636.3413-4-masahisa.kojima@linaro.org> MIME-Version: 1.0 In-Reply-To: <20190722115636.3413-4-masahisa.kojima@linaro.org> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > Signed-off-by: Satoru Okamoto > --- > .../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 >