[PATCH] sata_mv: increate the IORDY timeout for the soc controllers

[PATCH] sata_mv: increate the IORDY timeout for the soc controllers

[Martin Michlmayr]

From: Saeed Bishara <saeed@marvell.com>

The old value (in clock cycles) is not suitable for soc devices that
have internal clock more that 150MHz.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
Signed-off-by: Martin Michlmayr <tbm@cyrius.com>

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 5315b5f..f19f2e3 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -3330,7 +3330,7 @@ static void mv_soc_reset_hc_port(struct mv_host_priv *hpriv,
 	ZERO(0x024);		/* respq outp */
 	ZERO(0x020);		/* respq inp */
 	ZERO(0x02c);		/* test control */
-	writel(0xbc, port_mmio + EDMA_IORDY_TMOUT);
+	writel(0x800, port_mmio + EDMA_IORDY_TMOUT);
 }
 
 #undef ZERO

-- 
Martin Michlmayr
http://www.cyrius.com/

Re: [PATCH] sata_mv: increate the IORDY timeout for the soc controllers

[Mark Lord]

Martin Michlmayr wrote:
> From: Saeed Bishara <saeed@marvell.com>
> 
> The old value (in clock cycles) is not suitable for soc devices that
> have internal clock more that 150MHz.
> 
> Signed-off-by: Saeed Bishara <saeed@marvell.com>
> Signed-off-by: Martin Michlmayr <tbm@cyrius.com>
> 
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index 5315b5f..f19f2e3 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -3330,7 +3330,7 @@ static void mv_soc_reset_hc_port(struct mv_host_priv *hpriv,
>  	ZERO(0x024);		/* respq outp */
>  	ZERO(0x020);		/* respq inp */
>  	ZERO(0x02c);		/* test control */
> -	writel(0xbc, port_mmio + EDMA_IORDY_TMOUT);
> +	writel(0x800, port_mmio + EDMA_IORDY_TMOUT);
>  }
>  
>  #undef ZERO
..

I want to see somebody actually test/verify that
this works with PIO/MWDMA2 devices (eg. CF cards) first.

Test results?

Re: [PATCH] sata_mv: increate the IORDY timeout for the soc controllers

[saeed bishara]

On Sun, Apr 12, 2009 at 5:37 PM, Mark Lord <liml@rtr.ca> wrote:
> Martin Michlmayr wrote:
>>
>> From: Saeed Bishara <saeed@marvell.com>

>
> I want to see somebody actually test/verify that
> this works with PIO/MWDMA2 devices (eg. CF cards) first.
>
> Test results?
PIO mode is always used, at least for the identify command. the patch
works and needed for the Kirkwood SoC which runs at 200KHz.


saeed

Re: [PATCH] sata_mv: increate the IORDY timeout for the soc controllers

[Mark Lord]

saeed bishara wrote:
> On Sun, Apr 12, 2009 at 5:37 PM, Mark Lord <liml@rtr.ca> wrote:
>> Martin Michlmayr wrote:
>>> From: Saeed Bishara <saeed@marvell.com>
> 
>> I want to see somebody actually test/verify that
>> this works with PIO/MWDMA2 devices (eg. CF cards) first.
>>
>> Test results?
> PIO mode is always used, at least for the identify command. the patch
> works and needed for the Kirkwood SoC which runs at 200KHz.
..

Errr.. IDENTIFY is often a "special case" command,
and rarely, if ever, uses IORDY.

How about some other result?

The past couple of patches from you before these *broke* the driver,
which is why I'd like to see this and future patches verified first.

Thanks.

Re: [PATCH] sata_mv: increate the IORDY timeout for the soc controllers

[saeed bishara]

>>
>> PIO mode is always used, at least for the identify command. the patch
>> works and needed for the Kirkwood SoC which runs at 200KHz.
>
> ..
>
> Errr.. IDENTIFY is often a "special case" command,
> and rarely, if ever, uses IORDY.
I think you misunderstand the EDMA IORDY timer, The EDMA IORDY
register defines the timeout when doing PIO data transfers and the
data is not ready, for example, when doing PIO read, the CPU will
access the data register to fetch the data from internal fifo, in case
that fifo is emptied for too long time (the data stopped to come), the
HW will release the CPU to prevent hanging the system.


saeed

Re: [PATCH] sata_mv: increate the IORDY timeout for the soc controllers

[Mark Lord]

saeed bishara wrote:
>>> PIO mode is always used, at least for the identify command. the patch
>>> works and needed for the Kirkwood SoC which runs at 200KHz.
>> ..
>>
>> Errr.. IDENTIFY is often a "special case" command,
>> and rarely, if ever, uses IORDY.
> I think you misunderstand the EDMA IORDY timer
..

Actually, I've been quite familiar with IORDY since 1992 or so.
It generally is only asserted when the drive is managing some
form of elastic buffer between the media and the ATA interface.

In the case of IDENTIFY, many drives will not need to access
the media (though some do), and as a result IORDY won't be used
to stretch the access cycles.

A far better test for your IORDY patch would be to actually use
PIO read/write commands to verify that it works correctly.

Cheers