[U-Boot] [PATCH] mvsata_ide: adjust port init sequence
mvsata_ide_initialize_port(): adjust init sequence (SStatus should be checked only after all writes to SControl) and return success/failure to ide_preinit().
Also, as some tests showed init durations in the hundreds of us, raise the time-out to 01 ms to be on the safe side. --- drivers/block/mvsata_ide.c | 22 ++++++++++++++-------- 1 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/block/mvsata_ide.c b/drivers/block/mvsata_ide.c index 077b278..2b6b706 100644 --- a/drivers/block/mvsata_ide.c +++ b/drivers/block/mvsata_ide.c @@ -97,23 +97,27 @@ struct mvsata_port_registers { * DET back to "no action". */
-static void mvsata_ide_initialize_port(struct mvsata_port_registers *port) +static int mvsata_ide_initialize_port(struct mvsata_port_registers *port) { u32 control; u32 status; - u32 tout = 50; /* wait at most 50 us for SATA reset to complete */ + u32 tout = 10000; /* wait at most 10 ms for SATA reset to complete */
+ /* Set control IPM to 3 (no low power) and DET to 1 (initialize) */ control = readl(&port->scontrol); control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_INIT; + /* Toggle control DET back to 0 (normal operation) */ writel(control, &port->scontrol); + control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_USE; + writel(control, &port->scontrol); + /* wait for status DET to become 3 (device and communication OK) */ while (--tout) { status = readl(&port->sstatus) & MVSATA_SSTATUS_DET_MASK; if (status == MVSATA_SSTATUS_DET_DEVCOMM) break; udelay(1); } - control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_USE; - writel(control, &port->scontrol); + return (tout? 0: 1); }
/* @@ -125,15 +129,17 @@ int ide_preinit(void) { /* Enable ATA port 0 (could be SATA port 0 or 1) if declared */ #if defined(CONFIG_SYS_ATA_IDE0_OFFSET) - mvsata_ide_initialize_port( + if (mvsata_ide_initialize_port( (struct mvsata_port_registers *) - (CONFIG_SYS_ATA_BASE_ADDR + CONFIG_SYS_ATA_IDE0_OFFSET)); + (CONFIG_SYS_ATA_BASE_ADDR + CONFIG_SYS_ATA_IDE0_OFFSET))) + return 1; #endif /* Enable ATA port 1 (could be SATA port 0 or 1) if declared */ #if defined(CONFIG_SYS_ATA_IDE1_OFFSET) - mvsata_ide_initialize_port( + if (mvsata_ide_initialize_port( (struct mvsata_port_registers *) - (CONFIG_SYS_ATA_BASE_ADDR + CONFIG_SYS_ATA_IDE1_OFFSET)); + (CONFIG_SYS_ATA_BASE_ADDR + CONFIG_SYS_ATA_IDE1_OFFSET))) + return 1; #endif /* return 0 as we always succeed */ return 0;
Hello.
On 04-09-2010 14:34, Albert Aribaud wrote:
mvsata_ide_initialize_port(): adjust init sequence (SStatus should be checked only after all writes to SControl) and return success/failure to ide_preinit().
Also, as some tests showed init durations in the hundreds of us, raise the time-out to 01 ms to be on the safe side.
drivers/block/mvsata_ide.c | 22 ++++++++++++++-------- 1 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/block/mvsata_ide.c b/drivers/block/mvsata_ide.c index 077b278..2b6b706 100644 --- a/drivers/block/mvsata_ide.c +++ b/drivers/block/mvsata_ide.c @@ -97,23 +97,27 @@ struct mvsata_port_registers {
- DET back to "no action".
*/
-static void mvsata_ide_initialize_port(struct mvsata_port_registers *port) +static int mvsata_ide_initialize_port(struct mvsata_port_registers *port) { u32 control; u32 status;
- u32 tout = 50; /* wait at most 50 us for SATA reset to complete */
u32 tout = 10000; /* wait at most 10 ms for SATA reset to complete */
/* Set control IPM to 3 (no low power) and DET to 1 (initialize) */ control = readl(&port->scontrol); control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_INIT;
/* Toggle control DET back to 0 (normal operation) */
But you haven't set it to 1 yet. Maybe the comment is misplaced?
writel(control, &port->scontrol);
- control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_USE;
- writel(control, &port->scontrol);
- /* wait for status DET to become 3 (device and communication OK) */ while (--tout) { status = readl(&port->sstatus)& MVSATA_SSTATUS_DET_MASK; if (status == MVSATA_SSTATUS_DET_DEVCOMM) break; udelay(1); }
- control = (control& ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_USE;
- writel(control,&port->scontrol);
- return (tout? 0: 1);
Why not !tout? (And you forgot a space before colon. :-)
WBR, Sergei
Hi Sergei,
Le 05/09/2010 12:54, Sergei Shtylyov a écrit :
- /* Set control IPM to 3 (no low power) and DET to 1 (initialize) */
control = readl(&port->scontrol); control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_INIT;
- /* Toggle control DET back to 0 (normal operation) */
But you haven't set it to 1 yet. Maybe the comment is misplaced?
Look at the comment above, which said DET was set to 1, and at MVSATA_PORT_INIT, which combines IPM=3 and DET=1.
- return (tout? 0: 1);
Why not !tout? (And you forgot a space before colon. :-)
Why not indeed? I'll fix this in the next version.
WBR, Sergei
Thanks!
Amicalement,
Hello.
On 05.09.2010 23:33, Albert ARIBAUD wrote:
- /* Set control IPM to 3 (no low power) and DET to 1 (initialize) */
control = readl(&port->scontrol); control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_INIT;
- /* Toggle control DET back to 0 (normal operation) */
But you haven't set it to 1 yet. Maybe the comment is misplaced?
Look at the comment above, which said DET was set to 1, and at MVSATA_PORT_INIT, which combines IPM=3 and DET=1.
But you didn't write that value yet before your comment saying that you're resetting DET back to 0.
WBR, Sergei
Le 06/09/2010 11:18, Sergei Shtylyov a écrit :
Hello.
On 05.09.2010 23:33, Albert ARIBAUD wrote:
- /* Set control IPM to 3 (no low power) and DET to 1 (initialize) */
control = readl(&port->scontrol); control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_INIT;
- /* Toggle control DET back to 0 (normal operation) */
But you haven't set it to 1 yet. Maybe the comment is misplaced?
Look at the comment above, which said DET was set to 1, and at MVSATA_PORT_INIT, which combines IPM=3 and DET=1.
But you didn't write that value yet before your comment saying that you're resetting DET back to 0.
Yes, I did:
#define MVSATA_PORT_INIT \ (MVSATA_SCONTROL_DET_INIT|MVSATA_SCONTROL_IPM_NO_LP_ALLOWED)
MVSATA_PORT_INIT has both IPM = 3 (no low power allowed) and DET = 1 (initialize).
Amicalement,
Hello.
Albert ARIBAUD wrote:
On 05.09.2010 23:33, Albert ARIBAUD wrote:
- /* Set control IPM to 3 (no low power) and DET to 1 (initialize) */
control = readl(&port->scontrol); control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_INIT;
- /* Toggle control DET back to 0 (normal operation) */
But you haven't set it to 1 yet. Maybe the comment is misplaced?
Look at the comment above, which said DET was set to 1, and at MVSATA_PORT_INIT, which combines IPM=3 and DET=1.
But you didn't write that value yet before your comment saying that you're resetting DET back to 0.
Yes, I did:
#define MVSATA_PORT_INIT \ (MVSATA_SCONTROL_DET_INIT|MVSATA_SCONTROL_IPM_NO_LP_ALLOWED)
MVSATA_PORT_INIT has both IPM = 3 (no low power allowed) and DET = 1 (initialize).
Sigh. Your "resetting DET" comment is before the first writel().
Amicalement,
WBR, Sergei
Le 06/09/2010 14:45, Sergei Shtylyov a écrit :
Hello.
Albert ARIBAUD wrote:
On 05.09.2010 23:33, Albert ARIBAUD wrote:
- /* Set control IPM to 3 (no low power) and DET to 1 (initialize) */
control = readl(&port->scontrol); control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_INIT;
- /* Toggle control DET back to 0 (normal operation) */
But you haven't set it to 1 yet. Maybe the comment is misplaced?
Look at the comment above, which said DET was set to 1, and at MVSATA_PORT_INIT, which combines IPM=3 and DET=1.
But you didn't write that value yet before your comment saying that you're resetting DET back to 0.
Yes, I did:
#define MVSATA_PORT_INIT \ (MVSATA_SCONTROL_DET_INIT|MVSATA_SCONTROL_IPM_NO_LP_ALLOWED)
MVSATA_PORT_INIT has both IPM = 3 (no low power allowed) and DET = 1 (initialize).
Sigh. Your "resetting DET" comment is before the first writel().
Amicalement,
WBR, Sergei
Oh, I see! I'll push a new version with the comment moved down one notch. Thanks for your patience. :)
Amicalement,
participants (3)
-
Albert ARIBAUD -
Albert Aribaud
-
Sergei Shtylyov