public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Zeng, Star" <star.zeng@intel.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Tian, Feng" <feng.tian@intel.com>,
	"Dong, Eric" <eric.dong@intel.com>,
	"leif.lindholm@linaro.org" <leif.lindholm@linaro.org>,
	"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH v2] MdeModulePkg/AtaAtapiPassThru: relax PHY detect timeout
Date: Wed, 28 Jun 2017 08:31:19 +0000	[thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B8EF571@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <20170628082323.30270-1-ard.biesheuvel@linaro.org>

The updated comments "Wait at least 10 ms" seems not correct.


Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Wednesday, June 28, 2017 4:23 PM
To: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; Dong, Eric <eric.dong@intel.com>; leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [edk2] [PATCH v2] 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>
---
v2: update comment in AhciModeInitialization() as well

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

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index 4d01c1dd7fca..4418e5c3763e 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -2376,8 +2376,9 @@ 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 at least 10 ms for the Phy to detect the presence of a device.
+      // It's the requirement from SATA1.0a spec section 5.2.
+      // Add a bit of margin for robustness.
       //
       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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


  reply	other threads:[~2017-06-28  8:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-28  8:23 [PATCH v2] MdeModulePkg/AtaAtapiPassThru: relax PHY detect timeout Ard Biesheuvel
2017-06-28  8:31 ` Zeng, Star [this message]
2017-06-28  8:43   ` Ard Biesheuvel
     [not found]     ` <7F1BAD85ADEA444D97065A60D2E97EE569A14A33@SHSMSX101.ccr.corp.intel.com>
2017-06-28  8:58       ` Ard Biesheuvel
2017-06-28  9:53         ` Zeng, Star
2017-06-28  9:58           ` Ard Biesheuvel
2017-06-28 10:44             ` Zeng, Star
2017-06-28 10:44               ` Ard Biesheuvel

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=0C09AFA07DD0434D9E2A0C6AEB0483103B8EF571@shsmsx102.ccr.corp.intel.com \
    --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