public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3] MdeModulePkg/AtaAtapiPassThru: relax PHY detect timeout
@ 2017-06-28 10:51 Ard Biesheuvel
  2017-06-28 10:55 ` Zeng, Star
  0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2017-06-28 10:51 UTC (permalink / raw)
  To: edk2-devel, star.zeng, eric.dong; +Cc: feng.tian, leif.lindholm, Ard Biesheuvel

The SATA spec mandates that link detection by the PHY completes within
10 ms after receiving a reset signal. However, there is no obligation
to uphold this requirement at the driver end as strictly as we do, and
as it turns out, some combinations of host and device (e.g., Samsung
850 EVO connected to a LeMaker Cello) are only borderline compliant,
which means the device is not detected reliably.

So let's allow for a bit of margin, and increase the PHY detect timeout
value to 15 ms.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v3: update comment AhciModeInitialization() again

 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 3 +--
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h | 3 ++-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index 4d01c1dd7fca..b954de81015e 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -2376,8 +2376,7 @@ AhciModeInitialization (
       AhciOrReg (PciIo, Offset, EFI_AHCI_PORT_CMD_FRE);
 
       //
-      // Wait no longer than 10 ms to wait the Phy to detect the presence of a device.
-      // It's the requirment from SATA1.0a spec section 5.2.
+      // Wait for the Phy to detect the presence of a device.
       //
       PhyDetectDelay = EFI_AHCI_BUS_PHY_DETECT_TIMEOUT;
       Offset = EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH + EFI_AHCI_PORT_SSTS;
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
index 6401fb2e9fcd..809bcc307fc4 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
@@ -41,8 +41,9 @@ typedef union {
 
 //
 // Refer SATA1.0a spec section 5.2, the Phy detection time should be less than 10ms.
+// Add a bit of margin for robustness.
 //
-#define  EFI_AHCI_BUS_PHY_DETECT_TIMEOUT       10
+#define  EFI_AHCI_BUS_PHY_DETECT_TIMEOUT       15
 //
 // Refer SATA1.0a spec, the FIS enable time should be less than 500ms.
 //
-- 
2.9.3



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

* Re: [PATCH v3] MdeModulePkg/AtaAtapiPassThru: relax PHY detect timeout
  2017-06-28 10:51 [PATCH v3] MdeModulePkg/AtaAtapiPassThru: relax PHY detect timeout Ard Biesheuvel
@ 2017-06-28 10:55 ` Zeng, Star
  2017-06-28 12:48   ` Ard Biesheuvel
  0 siblings, 1 reply; 3+ messages in thread
From: Zeng, Star @ 2017-06-28 10:55 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org, Dong, Eric
  Cc: Tian, Feng, leif.lindholm@linaro.org, Zeng, Star

Reviewed-by: Star Zeng <star.zeng@intel.com>

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Wednesday, June 28, 2017 6:51 PM
To: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH v3] MdeModulePkg/AtaAtapiPassThru: relax PHY detect timeout

The SATA spec mandates that link detection by the PHY completes within
10 ms after receiving a reset signal. However, there is no obligation to uphold this requirement at the driver end as strictly as we do, and as it turns out, some combinations of host and device (e.g., Samsung
850 EVO connected to a LeMaker Cello) are only borderline compliant, which means the device is not detected reliably.

So let's allow for a bit of margin, and increase the PHY detect timeout value to 15 ms.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v3: update comment AhciModeInitialization() again

 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 3 +--  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h | 3 ++-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index 4d01c1dd7fca..b954de81015e 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -2376,8 +2376,7 @@ AhciModeInitialization (
       AhciOrReg (PciIo, Offset, EFI_AHCI_PORT_CMD_FRE);
 
       //
-      // Wait no longer than 10 ms to wait the Phy to detect the presence of a device.
-      // It's the requirment from SATA1.0a spec section 5.2.
+      // Wait for the Phy to detect the presence of a device.
       //
       PhyDetectDelay = EFI_AHCI_BUS_PHY_DETECT_TIMEOUT;
       Offset = EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH + EFI_AHCI_PORT_SSTS; diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
index 6401fb2e9fcd..809bcc307fc4 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
@@ -41,8 +41,9 @@ typedef union {
 
 //
 // Refer SATA1.0a spec section 5.2, the Phy detection time should be less than 10ms.
+// Add a bit of margin for robustness.
 //
-#define  EFI_AHCI_BUS_PHY_DETECT_TIMEOUT       10
+#define  EFI_AHCI_BUS_PHY_DETECT_TIMEOUT       15
 //
 // Refer SATA1.0a spec, the FIS enable time should be less than 500ms.
 //
--
2.9.3



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

* Re: [PATCH v3] MdeModulePkg/AtaAtapiPassThru: relax PHY detect timeout
  2017-06-28 10:55 ` Zeng, Star
@ 2017-06-28 12:48   ` Ard Biesheuvel
  0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2017-06-28 12:48 UTC (permalink / raw)
  To: Zeng, Star
  Cc: edk2-devel@lists.01.org, Dong, Eric, Tian, Feng,
	leif.lindholm@linaro.org

On 28 June 2017 at 10:55, Zeng, Star <star.zeng@intel.com> wrote:
> Reviewed-by: Star Zeng <star.zeng@intel.com>
>

Thanks

Pushed as 1fb805b1eb5b

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, June 28, 2017 6:51 PM
> To: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>
> Cc: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [PATCH v3] MdeModulePkg/AtaAtapiPassThru: relax PHY detect timeout
>
> The SATA spec mandates that link detection by the PHY completes within
> 10 ms after receiving a reset signal. However, there is no obligation to uphold this requirement at the driver end as strictly as we do, and as it turns out, some combinations of host and device (e.g., Samsung
> 850 EVO connected to a LeMaker Cello) are only borderline compliant, which means the device is not detected reliably.
>
> So let's allow for a bit of margin, and increase the PHY detect timeout value to 15 ms.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> v3: update comment AhciModeInitialization() again
>
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 3 +--  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h | 3 ++-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> index 4d01c1dd7fca..b954de81015e 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> @@ -2376,8 +2376,7 @@ AhciModeInitialization (
>        AhciOrReg (PciIo, Offset, EFI_AHCI_PORT_CMD_FRE);
>
>        //
> -      // Wait no longer than 10 ms to wait the Phy to detect the presence of a device.
> -      // It's the requirment from SATA1.0a spec section 5.2.
> +      // Wait for the Phy to detect the presence of a device.
>        //
>        PhyDetectDelay = EFI_AHCI_BUS_PHY_DETECT_TIMEOUT;
>        Offset = EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH + EFI_AHCI_PORT_SSTS; diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> index 6401fb2e9fcd..809bcc307fc4 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> @@ -41,8 +41,9 @@ typedef union {
>
>  //
>  // Refer SATA1.0a spec section 5.2, the Phy detection time should be less than 10ms.
> +// Add a bit of margin for robustness.
>  //
> -#define  EFI_AHCI_BUS_PHY_DETECT_TIMEOUT       10
> +#define  EFI_AHCI_BUS_PHY_DETECT_TIMEOUT       15
>  //
>  // Refer SATA1.0a spec, the FIS enable time should be less than 500ms.
>  //
> --
> 2.9.3
>


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

end of thread, other threads:[~2017-06-28 12:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-28 10:51 [PATCH v3] MdeModulePkg/AtaAtapiPassThru: relax PHY detect timeout Ard Biesheuvel
2017-06-28 10:55 ` Zeng, Star
2017-06-28 12:48   ` Ard Biesheuvel

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