Re: [edk2-devel] [edk2-platforms PATCH v1 3/3] NetsecDxe: SnpInitialize() waits for media linking up
On Wed, 24 Jul 2019 at 01:14, Leif Lindholm 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
> > 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)
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|0x0808
> >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_tphy_link_status;
> > + ogma_gmac_mode_t ogma_gmac_mode;
> > + ogma_err_togma_err;
> > + BOOLEAN ValidFlag;
> > + ogma_gmac_mode_t GmacMode;
> > + BOOLEAN RxRunningFlag;
> > + BOOLEAN TxRunningFlag;
> > + EFI_STATUSErrorStatus;
> > +
> > + LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
> > +
> > + // Update the media status
> > + ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> > + _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, ,
> > ,
> > + , );
> > + 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 (_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);
> > +
Re: [edk2-devel] [edk2-platforms PATCH v1 3/3] NetsecDxe: SnpInitialize() waits for media linking up
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|0x0808
>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_tphy_link_status;
> + ogma_gmac_mode_t ogma_gmac_mode;
> + ogma_err_togma_err;
> + BOOLEAN ValidFlag;
> + ogma_gmac_mode_t GmacMode;
> + BOOLEAN RxRunningFlag;
> + BOOLEAN TxRunningFlag;
> + EFI_STATUSErrorStatus;
> +
> + LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
> +
> + // Update the media status
> + ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
> + _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, , ,
> + , );
> + 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 (_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, _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;
> +
[edk2-devel] [edk2-platforms PATCH v1 3/3] NetsecDxe: SnpInitialize() waits for media linking up
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(-)
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|0x0808
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_tphy_link_status;
+ ogma_gmac_mode_t ogma_gmac_mode;
+ ogma_err_togma_err;
+ BOOLEAN ValidFlag;
+ ogma_gmac_mode_t GmacMode;
+ BOOLEAN RxRunningFlag;
+ BOOLEAN TxRunningFlag;
+ EFI_STATUSErrorStatus;
+
+ LanDriver = INSTANCE_FROM_SNP_THIS (Snp);
+
+ // Update the media status
+ ogma_err = ogma_get_phy_link_status (LanDriver->Handle,
+ _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, , ,
+ , );
+ 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 (_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, _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
