public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Tony Dinh <mibodhi@gmail.com>
To: Stefan Roese <sr@denx.de>, U-Boot Mailing List <u-boot@lists.denx.de>
Cc: Tom Rini <trini@konsulko.com>, Simon Glass <sjg@chromium.org>,
	Tony Dinh <mibodhi@gmail.com>
Subject: [PATCH v2] arm: mvebu: sata_mv failed to identify HDDs during cold start
Date: Sat, 31 Jul 2021 20:29:35 -0700	[thread overview]
Message-ID: <20210801032935.5533-1-mibodhi@gmail.com> (raw)
In-Reply-To: <26c76127-7415-d3dd-f20d-ac05cd04a6b0@denx.de>

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.

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.

Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Tony Dinh <mibodhi@gmail.com>
---

Changes in v2:
Return value -ENODEV if mv_identify() failed to identify the drive

 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..dadb2c7c2e 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 -ENODEV;
 }
 
 static void mv_sata_xfer_mode(struct udevice *dev, int port, u16 *id)
-- 
2.20.1


  parent reply	other threads:[~2021-08-01  3:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-25 21:57 [PATCH] arm: mvebu: sata_mv failed to identify HDDs during cold start Tony Dinh
2021-07-31  7:31 ` Stefan Roese
2021-07-31  9:28   ` Tony Dinh
2021-08-01  3:29   ` Tony Dinh [this message]
2021-08-11  8:28     ` [PATCH v2] " Stefan Roese

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-all 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=20210801032935.5533-1-mibodhi@gmail.com \
    --to=mibodhi@gmail.com \
    --cc=sjg@chromium.org \
    --cc=sr@denx.de \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /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