On Wed, 24 Jul 2019 at 01:14, Leif Lindholm <leif.lindh...@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.koj...@linaro.org>
> > Signed-off-by: Satoru Okamoto <okamoto.sat...@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
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44286): https://edk2.groups.io/g/devel/message/44286
Mute This Topic: https://groups.io/mt/32557833/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
