From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x22c.google.com (mail-io0-x22c.google.com [IPv6:2607:f8b0:4001:c06::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0EFA72095D213 for ; Wed, 28 Jun 2017 01:41:31 -0700 (PDT) Received: by mail-io0-x22c.google.com with SMTP id h134so31776855iof.2 for ; Wed, 28 Jun 2017 01:43:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=fZiRt6lXKlpkcWfZ6qAODaMbXgjvdV3DOv0TnjQyfuA=; b=Pzzp8npMbN5jo7faqKzLYHcudJfOoHdCg6dZAyCNAbU4w9LrtNmniYdLFp8c0/kDSH JtyuhfFcx9JOkLAPapa0E3YPeDskqHcTScTSOQr81Ja0fGqQ/bIsFUyaxCc/cQiY91IT U+3RfUMLvMGy0sb2Ek/O/vyswv/KCO/TdG5Uw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=fZiRt6lXKlpkcWfZ6qAODaMbXgjvdV3DOv0TnjQyfuA=; b=dT1Lbgasmx8wUC9k+tc+OBu3emCIwQKdVZPD2FGaoW4OV4PxeIfc/a7tVpoX7UJuzh NslSOnkAiUbMtxvCEYW2JbvCwFqvt5X39nX1OAI7DYNq1j7plk/dGBTl7ML9D/tBKRI9 ciBFCQjZq9j9SVeBaGiNXjTuTaDULkrIgXEEG2Y/h4L8PtuMFeAZ+hfZlc9Lqs+DYmHb 2MbvNXIw0b8OgDeDj+2Yz8S4CEYuX8iuoUYyJrYFf7BQHNBZmwTynl7gYBc54A5QDosl rEeJtvPZwcfFG0J7Wap6o8GbTH1oeHZxDxHIzfbQu+l4W+QoBdYkV0YkvXyDxD4KRccE r3vg== X-Gm-Message-State: AKS2vOy4cBI8/bczE7f6PsFyXDxKNnbnX5O+owYoAG0tIHWDJWB5AXNF S5KXum1KLfxDTYHfzdWmfonL8h9vER8Y X-Received: by 10.107.180.20 with SMTP id d20mr10137128iof.47.1498639381710; Wed, 28 Jun 2017 01:43:01 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.134.134 with HTTP; Wed, 28 Jun 2017 01:43:01 -0700 (PDT) In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B8EF571@shsmsx102.ccr.corp.intel.com> References: <20170628082323.30270-1-ard.biesheuvel@linaro.org> <0C09AFA07DD0434D9E2A0C6AEB0483103B8EF571@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Wed, 28 Jun 2017 08:43:01 +0000 Message-ID: To: "Zeng, Star" Cc: "edk2-devel@lists.01.org" , "Tian, Feng" , "Dong, Eric" , "leif.lindholm@linaro.org" Subject: Re: [PATCH v2] MdeModulePkg/AtaAtapiPassThru: relax PHY detect timeout X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Jun 2017 08:41:31 -0000 Content-Type: text/plain; charset="UTF-8" On 28 June 2017 at 08:31, Zeng, Star wrote: > The updated comments "Wait at least 10 ms" seems not correct. > That is the whole point of the change. The SATA spec mandates that the PHY respond within 10 ms. It does *not* mandate that the software wait 10 ms or less, rather the opposite, i.e., that the software should wait 10 ms or *more*, so 'at least 10ms'. The original comment said that the software *must* wait for no more than 10 ms, but this is not what the spec says. > -----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 > Cc: Tian, Feng ; Dong, Eric ; leif.lindholm@linaro.org; Ard Biesheuvel > 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 > --- > 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