From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A8E74C4338F for ; Sat, 31 Jul 2021 07:32:07 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4D72060EBC for ; Sat, 31 Jul 2021 07:32:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4D72060EBC Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B7DD9831EB; Sat, 31 Jul 2021 09:32:02 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1627716723; bh=7jLn2DlCu4Z4Qe4sUa2fZT2D69Z9WFKEJgKV8c7sepY=; h=Subject:To:Cc:References:From:Date:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=FUsEL+yQ3UIazl4ZwlzD55kNiWgDv6EzLoWhfJZ2bEQdM0i3WXMZAu5Lb2rQPiaxW ObVkJZcoF5oSDdrLhx7/pGwdi29XWolFeaio2ASD4YpUJhy/geL4WYTU7pnF858UrB tpfZgFInvSB7v6LOkE4+PFTJQpDEHR8jpoZRRcsykVeQJfMXupYCiWxvKI70pir5gL LrKLukEi+7VHxk7faJQg53GL6VIZGqbNg5Z0ycDgvOm8bgs0kMfH/LzsqEUZ3h7C4c h+P8bfFDfQsYfOpAWGMsosCzIn76aG+3KIzKLAx3BjCRJGlGLTxGdG5sIFpTxTd5GX H/jAz/A+YIJxw== Received: by phobos.denx.de (Postfix, from userid 109) id 900B283223; Sat, 31 Jul 2021 09:32:00 +0200 (CEST) Received: from mout-u-107.mailbox.org (mout-u-107.mailbox.org [91.198.250.252]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 2A1E1831EB for ; Sat, 31 Jul 2021 09:31:57 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=sr@denx.de Received: from smtp1.mailbox.org (smtp1.mailbox.org [80.241.60.240]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-u-107.mailbox.org (Postfix) with ESMTPS id 4GcGDY03vBzQk93; Sat, 31 Jul 2021 09:31:57 +0200 (CEST) Received: from smtp1.mailbox.org ([80.241.60.240]) by hefe.heinlein-support.de (hefe.heinlein-support.de [91.198.250.172]) (amavisd-new, port 10030) with ESMTP id IDqd_iaEm7Tz; Sat, 31 Jul 2021 09:31:53 +0200 (CEST) Subject: Re: [PATCH] arm: mvebu: sata_mv failed to identify HDDs during cold start To: Tony Dinh , U-Boot Mailing List Cc: Simon Glass , Tom Rini References: <20210725215751.26295-1-mibodhi@gmail.com> From: Stefan Roese Message-ID: <26c76127-7415-d3dd-f20d-ac05cd04a6b0@denx.de> Date: Sat, 31 Jul 2021 09:31:53 +0200 MIME-Version: 1.0 In-Reply-To: <20210725215751.26295-1-mibodhi@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: de-DE Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 09CAD18B7 X-Rspamd-UID: 7be2f2 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi Tony, On 25.07.21 23:57, Tony Dinh wrote: > During cold start, with some HDDs, mv_sata_identify() does not populate > the ID words on the 1st ATA ID command. In fact, the first ATA ID > command will only power up the drive, and then the ATA ID command > processing is lost in the process. > > Tests with: > > - Seagate ST9250320AS 250GB HDD and Seagate ST4000DM004-2CV104 4TB HDD. > - Zyxel NSA310S (Kirkwood 88F6702), Marvell Dreamplug (Kirkwood 88F6281), > Seagate GoFlex Home (Kirkwood 88F6281), Pogoplug V4 (Kirkwood 88F6192). > > Observation: > > - The Seagate ST9250320AS 250GB took about 3 seconds to spin up. > - The Seagate ST4000DM004-2CV104 4TB took about 8 seconds to spin up. > - mv_sata_identify() did not populate the ID words after the call to > mv_ata_exec_ata_cmd_nondma(). > - Attempt to insert a long delay of 30 seconds, ie. mdelay(30_000), after > the call to ata_wait_register() inside mv_ata_exec_ata_cmd_nondma() did > not help with the 4TB drive. The ID words were still empty after that 30s > delay. IIRC, I've seen similar issues with a SATA SSD on theadorable before as well. Thanks for digging into this. One some nit below... > Patch Description: > > - Added a second ATA ID command in mv_sata_identify(), which will be > executed if the 1st ATA ID command did not return with valid ID words. > - Use the HDD drive capacity in the ID words as a successful indicator of > ATA ID command. > - In the scenario where a box is rebooted, the 1st ATA ID command is always > successful, so there is no extra time wasted. > - In the scenario where a box is cold started, the 1st ATA command is the > power up command. The 2nd ATA ID command alleviates the uncertainty of > how long we have to wait for the ID words to be populated by the SATA > controller. > > Signed-off-by: Tony Dinh > --- > > drivers/ata/sata_mv.c | 29 +++++++++++++++++++++++++++-- > 1 file changed, 27 insertions(+), 2 deletions(-) > > diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c > index 1012cb5374..7d1515d5f8 100644 > --- a/drivers/ata/sata_mv.c > +++ b/drivers/ata/sata_mv.c > @@ -809,6 +809,7 @@ static int mv_ata_exec_ata_cmd_nondma(struct udevice *dev, int port, > static int mv_sata_identify(struct udevice *dev, int port, u16 *id) > { > struct sata_fis_h2d h2d; > + int len; > > memset(&h2d, 0, sizeof(struct sata_fis_h2d)); > > @@ -818,8 +819,32 @@ static int mv_sata_identify(struct udevice *dev, int port, u16 *id) > /* Give device time to get operational */ > mdelay(10); > > - return mv_ata_exec_ata_cmd_nondma(dev, port, &h2d, (u8 *)id, > - ATA_ID_WORDS * 2, READ_CMD); > + /* During cold start, with some HDDs, the first ATA ID command does > + * not populate the ID words. In fact, the first ATA ID > + * command will only power up the drive, and then the ATA ID command > + * processing is lost in the process. > + */ > + len = mv_ata_exec_ata_cmd_nondma(dev, port, &h2d, (u8 *)id, > + ATA_ID_WORDS * 2, READ_CMD); > + > + /* If drive capacity has been filled in, then it was successfully > + * identified (the drive has been powered up before, i.e. > + * this function is invoked during a reboot) > + */ > + if (ata_id_n_sectors(id) != 0) > + return len; > + > + /* Issue the 2nd ATA ID command to make sure the ID words are > + * populated properly. > + */ > + mdelay(10); > + len = mv_ata_exec_ata_cmd_nondma(dev, port, &h2d, (u8 *)id, > + ATA_ID_WORDS * 2, READ_CMD); > + if (ata_id_n_sectors(id) != 0) > + return len; > + > + printf("Err: Failed to identify SATA device %d\n", port); > + return -1; Perhaps better to return -ENODEV or something similar instead of the -1 here? Thanks, Stefan