[PATCH] libata: add 150ms between completion of hardreset and status checking

[PATCH] libata: add 150ms between completion of hardreset and status checking

[Tejun Heo]

Follow the old SRST rule and delay 150ms between completion of
hardreset and status checking.  Debouncing delay should usually cover
this but debounce duration could be shorter than 150ms under certain
circumstances.

Usefulness depends on host controller implementation but it can't hurt
and serves as a reminder that 2s delay for GoVault should also be
added here.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-core.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index f210dbd..582e44d 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3014,6 +3014,9 @@ int sata_std_hardreset(struct ata_port *ap, unsigned int *class)
 		return 0;
 	}
 
+	/* wait a while before checking status, see SRST for more info */
+	msleep(150);
+
 	if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
 		ata_port_printk(ap, KERN_ERR,
 				"COMRESET failed (device not ready)\n");
-- 
1.4.4.4

Re: [PATCH] libata: add 150ms between completion of hardreset and status checking

[Bob Stewart]

Tejun,

There is still something else bad going on in your driver.  I've tried
everything I can think of (timing changes, flushes, and whatnot)
but I still get random timeouts and random results from md5sum
on large files.  Even in the case where I don't get a timeout on a large
iso file, the md5sum is still random.  Strangely enough, though, 
the files may be good on-disk, since totem seems to play the DVD.iso
that I'm testing with without a problem.  I'm using debian with a 2.6.19.2
kernel.

Bob


--- Tejun Heo <htejun@gmail.com> wrote:

> Follow the old SRST rule and delay 150ms between completion of
> hardreset and status checking.  Debouncing delay should usually cover
> this but debounce duration could be shorter than 150ms under certain
> circumstances.
> 
> Usefulness depends on host controller implementation but it can't hurt
> and serves as a reminder that 2s delay for GoVault should also be
> added here.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
>  drivers/ata/libata-core.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index f210dbd..582e44d 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -3014,6 +3014,9 @@ int sata_std_hardreset(struct ata_port *ap, unsigned
> int *class)
>  		return 0;
>  	}
>  
> +	/* wait a while before checking status, see SRST for more info */
> +	msleep(150);
> +
>  	if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
>  		ata_port_printk(ap, KERN_ERR,
>  				"COMRESET failed (device not ready)\n");
> -- 
> 1.4.4.4
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] libata: add 150ms between completion of hardreset and status checking

[Tejun Heo]

[this thread is regarding sata_inic162x, misplaced reply]

Bob Stewart wrote:
> There is still something else bad going on in your driver.  I've tried
> everything I can think of (timing changes, flushes, and whatnot)
> but I still get random timeouts and random results from md5sum
> on large files.  Even in the case where I don't get a timeout on a large
> iso file, the md5sum is still random.  Strangely enough, though, 
> the files may be good on-disk, since totem seems to play the DVD.iso
> that I'm testing with without a problem.  I'm using debian with a 2.6.19.2
> kernel.

Yeap, I'm aware of the problem.  That's why the driver is marked HIGHLY
EXPERIMENTAL.  Maybe I should change it to broken.  :-(  Anyways, one of
the known problems is mishandling of LBA48 commands which is cause of
timeouts and wrong md5sums on large disks.  I just couldn't find any
time to work on it in recent weeks.  Please be patient a bit.  I'll get
back to sata_ini162x in a few days.

-- 
tejun

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Tejun Heo <htejun@gmail.com> wrote:

> [this thread is regarding sata_inic162x, misplaced reply]
> 
> Bob Stewart wrote:
> > There is still something else bad going on in your driver.  I've tried
> > everything I can think of (timing changes, flushes, and whatnot)
> > but I still get random timeouts and random results from md5sum
> > on large files.  Even in the case where I don't get a timeout on a large
> > iso file, the md5sum is still random.  Strangely enough, though, 
> > the files may be good on-disk, since totem seems to play the DVD.iso
> > that I'm testing with without a problem.  I'm using debian with a 2.6.19.2
> > kernel.
> 
> Yeap, I'm aware of the problem.  That's why the driver is marked HIGHLY
> EXPERIMENTAL.  Maybe I should change it to broken.  :-(  Anyways, one of
> the known problems is mishandling of LBA48 commands which is cause of
> timeouts and wrong md5sums on large disks.  I just couldn't find any
> time to work on it in recent weeks.  Please be patient a bit.  I'll get
> back to sata_ini162x in a few days.
> 
> -- 
> tejun
> 

Thanks Tejun,

I'm trying to use this as a learning tool, so I'm not putting pressure on you
to fix it.  Well, at least I've learned a bit about libata etc while digging
through 
this, even if I wasn't able to figure out what the problem was.  :)  By the
way, 
the drive I'm using to play with this now is only an 80GB drive.

Bob

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Mark Lord]

Bob Stewart wrote:
> By the way, 
> the drive I'm using to play with this now is only an 80GB drive.

Most newer drives are still capable of LBA48, even when they are
small enough to not require it for sector addressing.

If a large enough I/O (more than 256 sectors) is generated,
then libata may use LBA48 to complete a request on such drives.

Cheers

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Mark Lord <liml@rtr.ca> wrote:
> Bob Stewart wrote:
> > By the way, 
> > the drive I'm using to play with this now is only an 80GB drive.
> 
> Most newer drives are still capable of LBA48, even when they are
> small enough to not require it for sector addressing.
> 
> If a large enough I/O (more than 256 sectors) is generated,
> then libata may use LBA48 to complete a request on such drives.

Is there an easy way to tell whether the command was sent as 
LBA48, say, in the driver's bmdma_start function or even in the 
hardreset function after the fact?  One of my tests was using a 64 
sector limit, with pretty much the same results.  I always test against 
the same file, and the results, including the number of timeouts, are 
different from test to test.

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Mark Lord]

Bob Stewart wrote:
>
> Is there an easy way to tell whether the command was sent as 
> LBA48, say, in the driver's bmdma_start function or even in the 
> hardreset function after the fact?

The best thing would be to have libata log the ATA opcode,
if it doesn't already do that.

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Tejun Heo]

Hello,

Bob Stewart wrote:
> Is there an easy way to tell whether the command was sent as 
> LBA48, say, in the driver's bmdma_start function or even in the 
> hardreset function after the fact?  One of my tests was using a 64 
> sector limit, with pretty much the same results.  I always test against 
> the same file, and the results, including the number of timeouts, are 
> different from test to test.

The driver is seriously broken regarding LBA48 support.  The timeout
goes away if max_sectors is decreased to ATA_MAX_SECTORS - 1, doh.  But
both the reading and writing are seriously broken.  I can't tell whether
they end up in the wrong sectors or garbage is transferred to/from the
right sectors.

I'm really close to marking this device broken or we'll need to
implement a mechanism to veto LBA48 device from LLD (may be negative
return from ->dev_config).

Reading the sunix driver, I can't find any relevant workaround.  It
never seems to initialize ctl address.  I'm curious whether the sunix
driver can do LBA48.  Bob, are you interested in testing this?

Thanks.

-- 
tejun

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Tejun Heo <htejun@gmail.com> wrote:

> The driver is seriously broken regarding LBA48 support.  The timeout
> goes away if max_sectors is decreased to ATA_MAX_SECTORS - 1, doh.  But
> both the reading and writing are seriously broken.  I can't tell whether
> they end up in the wrong sectors or garbage is transferred to/from the
> right sectors.

I couldn't figure it out, either.  There were some times early on that I
failed it all the way down to PIO4 and it worked just fine.  But, with DMA
forget it.  

> I'm really close to marking this device broken or we'll need to
> implement a mechanism to veto LBA48 device from LLD (may be negative
> return from ->dev_config).

Are you sure it's an LBA48 issue?  I was never sure how to check
this but in my banging around, I never saw anything in the HOB fields.
 
> Reading the sunix driver, I can't find any relevant workaround.  It
> never seems to initialize ctl address.  I'm curious whether the sunix
> driver can do LBA48.  Bob, are you interested in testing this?

Sure.  I'll need to reorganize hardware again, but it'll give me something
to do till the steelhead start their spring run.  :)

Bob

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Tejun Heo]

Bob Stewart wrote:
> --- Tejun Heo <htejun@gmail.com> wrote:
> 
>> The driver is seriously broken regarding LBA48 support.  The timeout
>> goes away if max_sectors is decreased to ATA_MAX_SECTORS - 1, doh.  But
>> both the reading and writing are seriously broken.  I can't tell whether
>> they end up in the wrong sectors or garbage is transferred to/from the
>> right sectors.
> 
> I couldn't figure it out, either.  There were some times early on that I
> failed it all the way down to PIO4 and it worked just fine.  But, with DMA
> forget it.  

Hmmm... I played a bit with POLLING LBA48 but no luck.  I always got HSM
violation.

>> I'm really close to marking this device broken or we'll need to
>> implement a mechanism to veto LBA48 device from LLD (may be negative
>> return from ->dev_config).
> 
> Are you sure it's an LBA48 issue?  I was never sure how to check
> this but in my banging around, I never saw anything in the HOB fields.

Pretty sure.  If I write sequentially, the first sector out of LBA28
range gives me timeout (with 256 sector requests).  If I make a
filesystem insider LBA28 range proper, I can fsck it using another
controller and vice versa but if I do it outside of LBA48 range (using
255 sector requests), nothing gets written or read properly.

>> Reading the sunix driver, I can't find any relevant workaround.  It
>> never seems to initialize ctl address.  I'm curious whether the sunix
>> driver can do LBA48.  Bob, are you interested in testing this?
> 
> Sure.  I'll need to reorganize hardware again, but it'll give me something
> to do till the steelhead start their spring run.  :)

Have no idea what the steelhead is but thanks.  :-)

-- 
tejun

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Tejun Heo <htejun@gmail.com> wrote:

> Bob Stewart wrote:
> > --- Tejun Heo <htejun@gmail.com> wrote:
> > 
> >> The driver is seriously broken regarding LBA48 support.  The timeout
> >> goes away if max_sectors is decreased to ATA_MAX_SECTORS - 1, doh.  But
> >> both the reading and writing are seriously broken.  I can't tell whether
> >> they end up in the wrong sectors or garbage is transferred to/from the
> >> right sectors.
> > 
> > I couldn't figure it out, either.  There were some times early on that I
> > failed it all the way down to PIO4 and it worked just fine.  But, with DMA
> > forget it.  
> 
> Hmmm... I played a bit with POLLING LBA48 but no luck.  I always got HSM
> violation.

I spoke with imprecision.  What I meant was that I was able to get error-free
operation with PIO4 in LBA28, not with LBA48.  I have been testing using
md5sum on a 7+GB DVD ISO.  I have never gotten the same result twice 
using DMA.  I was able to get proper md5 sums with PIO, though I didn't
mess with that for too long.  The test drive was an 80GB WD800JB that
I've since moved to my "good" system.  I went ahead and ordered another
one to play with this some more.  It should be in in about 2 weeks.

BTW, did you want me to load a 2.4 kernel to test the sunix driver, or did you
have something else in mind?  I wasn't quite clear on what I was agreeing
to.  :)  I'm OK either way.  I've got a spare PATA drive to put the 2.4 kernel
on if necessary, and can test with the new WD800 on the INIC162x.  I've also
got a 200GB drive that I can do read testing with if it comes to an LBA48
issue.

(Steelhead are a species of rainbow trout.)

Bob

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Tejun Heo]

Bob Stewart wrote:
> BTW, did you want me to load a 2.4 kernel to test the sunix driver, or did you
> have something else in mind?  I wasn't quite clear on what I was agreeing
> to.  :)  I'm OK either way.  I've got a spare PATA drive to put the 2.4 kernel
> on if necessary, and can test with the new WD800 on the INIC162x.  I've also
> got a 200GB drive that I can do read testing with if it comes to an LBA48
> issue.

It would be great if you can test the sunix driver on 2.4 kernel for
both LBA28 and 48 cases.  I know it's a hassle and would really
appreciate it.

> (Steelhead are a species of rainbow trout.)

Man, I love trouts.  Wish I were there.  :-)

-- 
tejun

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Tejun Heo <htejun@gmail.com> wrote:
> It would be great if you can test the sunix driver on 2.4 kernel for
> both LBA28 and 48 cases.  I know it's a hassle and would really
> appreciate it.

Will do.  I'll be using  Debian Sarge 2.4.  It'll be 2 or 3 weeks before
I can do anything, though.  I'll post then.

Bob

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Bob Stewart <bob@evoria.net> wrote:
> --- Tejun Heo <htejun@gmail.com> wrote:
> > It would be great if you can test the sunix driver on 2.4 kernel for
> > both LBA28 and 48 cases.  I know it's a hassle and would really
> > appreciate it.
> 
> Will do.  I'll be using  Debian Sarge 2.4.  It'll be 2 or 3 weeks before
> I can do anything, though.  I'll post then.
> 
> Bob

Tejun,
I wonder if this card of mine is what's bad.  I loaded Sarge 2.4 and the
Sunix driver and a new disk onto my test whore.  I got essentially the 
same sort of errors as with your driver (see below) but in this case,
abolutely nothing was ever reliable, including the partition table.  I'm
not sure I want to buy another card to test this out, though.  Maybe 
I'll put the card in my other machine just to verify it's not a mobo 
issue, though.   crapcrapcrap!

Sample errors:

drive not ready for command
status timeout: status = 0x80 { busy }

error = 0x00 { }
dma_intr: status = 0x51 {drive ready SeekComplete error}

Bob

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Tejun Heo]

Bob Stewart wrote:
> I wonder if this card of mine is what's bad.  I loaded Sarge 2.4 and the
> Sunix driver and a new disk onto my test whore.  I got essentially the 
> same sort of errors as with your driver (see below) but in this case,
> abolutely nothing was ever reliable, including the partition table.  I'm
> not sure I want to buy another card to test this out, though.  Maybe 
> I'll put the card in my other machine just to verify it's not a mobo 
> issue, though.   crapcrapcrap!
> 
> Sample errors:
> 
> drive not ready for command
> status timeout: status = 0x80 { busy }
> 
> error = 0x00 { }
> dma_intr: status = 0x51 {drive ready SeekComplete error}

My bet is on broken driver.  sunix driver doesn't seem like it was 
written carefully.  Anyways, thanks for testing.  It seems we'll either 
have to implement adma mode.  I'll mark the driver broken for 2.6.21. 
Thanks.

-- 
tejun

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Tejun Heo <htejun@gmail.com> wrote:

> Bob Stewart wrote:
> > I wonder if this card of mine is what's bad.  I loaded Sarge 2.4 and the

> My bet is on broken driver.  sunix driver doesn't seem like it was 
> written carefully.  Anyways, thanks for testing.  It seems we'll either 
> have to implement adma mode.  I'll mark the driver broken for 2.6.21. 
> Thanks.
> -- 
> tejun

Well, I'll probably mess with trying to convert one of the adma drivers over
to work with this hardware.  But, I've never done this before, so it probably
won't come to much.

Bob

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Jeff Garzik]

Tejun Heo wrote:
> Bob Stewart wrote:
>> I wonder if this card of mine is what's bad.  I loaded Sarge 2.4 and the
>> Sunix driver and a new disk onto my test whore.  I got essentially the 
>> same sort of errors as with your driver (see below) but in this case,
>> abolutely nothing was ever reliable, including the partition table.  I'm
>> not sure I want to buy another card to test this out, though.  Maybe 
>> I'll put the card in my other machine just to verify it's not a mobo 
>> issue, though.   crapcrapcrap!
>>
>> Sample errors:
>>
>> drive not ready for command
>> status timeout: status = 0x80 { busy }
>>
>> error = 0x00 { }
>> dma_intr: status = 0x51 {drive ready SeekComplete error}
> 
> My bet is on broken driver.  sunix driver doesn't seem like it was 
> written carefully.  Anyways, thanks for testing.  It seems we'll either 
> have to implement adma mode.  I'll mark the driver broken for 2.6.21. 


Broken, or LBA28-only?

I would think that we have other controllers that need an LBA28-only 
flag.  (note: don't be fooled by ATA_FLAG_PIO_LBA48, that doesn't quite 
do what we want)

Alan, Mark, surely some ancient controller simply can't do LBA48 at all, 
even in PIO mode?

	Jeff

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Jeff Garzik <jeff@garzik.org> wrote:

> Alan, Mark, surely some ancient controller simply can't do LBA48 at all, 
> even in PIO mode?

Jeff,
Thanks for reminding me.  I need to retest Tejun's driver limited to PIO mode.
If I remember correctly, when I first started testing it and it failed down to
PIO4, it worked just fine on an LBA28 partition.

Bob

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Mark Lord]

Jeff Garzik wrote:
>..
> Broken, or LBA28-only?
> 
> I would think that we have other controllers that need an LBA28-only 
> flag.  (note: don't be fooled by ATA_FLAG_PIO_LBA48, that doesn't quite 
> do what we want)
> 
> Alan, Mark, surely some ancient controller simply can't do LBA48 at all, 
> even in PIO mode?

Mmm.. Alan has more docs in his possession than I do for those,
but I'd expect such beasts to be somewhat rare.

I haven't run into any here, but they do exist.

Cheers

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Alan Cox]

> > Alan, Mark, surely some ancient controller simply can't do LBA48 at all, 
> > even in PIO mode?
> 
> Mmm.. Alan has more docs in his possession than I do for those,
> but I'd expect such beasts to be somewhat rare.

The dumb controllers are generally ok. It's the "almost smart" ones and
the "I had some space left on my fpga and .." type controllers that tend
to break horribly.

Older promise needs ugly hacks to do LBA48 at all  (basically we lie to
the controller and set up for ATAPI), the Inic I suspect from the docs
does get it right in ADMAish mode as it has LBA48 sized command blocks.

One or two embedded devices just don't get the HOB stuff and the double
register feed right at all. LBA48 commands work but you get only half the
reply etc.

Older ALi just can't cope with LBA48 DMA as its own intelligence is just
the right level of half-smart (or half-dumb) to break. 

Also on the related TODO list is to add an "I'm too dumb to do DMA except
in sector size chunks" check_atapi_dma helper which should fix up old
promise a bit and perhaps ALi (whose FIFO engine also from playing
seems to need all the registers writing each command)

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Tejun Heo <htejun@gmail.com> wrote:

> 
> My bet is on broken driver.  sunix driver doesn't seem like it was 
> written carefully.  Anyways, thanks for testing.  It seems we'll either 
> have to implement adma mode.  I'll mark the driver broken for 2.6.21. 
> Thanks.

Tejun,

I did some more testing.  I limited the driver to PIO4 with the changes
shown below.  I copied a 2.2GB gzipped file from a local disk and a 7.9GB
iso file over my network onto the drive and then ran md5sum on both.  
The sums were fine under PIO4.  It's slow as hell, but at least it worked.  
If you have any suggestions for me to mess with in the driver, I'm up for it.

By the way, I'm now running 2.6.20.3 for my testing.  I had to wait to
upgrade till madwifi was ready.

        .pio_mask                  = 0x1f, /* pio0-4 */
        .mwdma_mask           = 0x00, /* mwdma0-2 */
        .udma_mask              = 0x00, /* udma0-6 */

Bob

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Bob Stewart <bob@evoria.net> wrote:
> Tejun,
> 
> I did some more testing.  I limited the driver to PIO4 with the changes
> shown below.  I copied a 2.2GB gzipped file from a local disk and a 7.9GB
> iso file over my network onto the drive and then ran md5sum on both.  
> The sums were fine under PIO4.  It's slow as hell, but at least it worked.  
> If you have any suggestions for me to mess with in the driver, I'm up for it.

Just one more bit of info.  I ran an md5sum against the 2.2G file in WMDMA2
mode and actually got the right result once.  Doesn't this, along with the
PIO4 working fine, and the driver working fine under light load, imply that
there's a race being lost somewhere?

Bob

Re: Old Thread: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

Tejun, 
This is an update to an old thread.

--- Tejun Heo <htejun@gmail.com> wrote:
> Bob Stewart wrote:
> > I wonder if this card of mine is what's bad.  <snipp.
> 
> My bet is on broken driver.  sunix driver doesn't seem like it was 
> written carefully.  Anyways, thanks for testing.  It seems we'll either 
> have to implement adma mode.  I'll mark the driver broken for 2.6.21. 
> Thanks.

I just got your note that there is a new SATA_INIC162X driver, so I
loaded up kernel 2.6.27-rc2 and gave it a try.  Timeouts again, so I
ordered a new card on ebay.  I'm sad to say that the errors, months
ago, were from a card that I had gotten bad from new.  Wasted a lot of my
time and this list's time.  Sorry about that.  :(

OTOH, with the new card, both your new driver and the old one seem
to be OK after several hours of copying ISO files and running "md5sum"
on the copied filed.  Good job!  Thanks for all the hard work!  If there are
any other tests needed on this driver let me know before I bow out again.

Bob

Re: Old Thread: sata_inic162x driver for 2.6.19 timeouts etc

[Tejun Heo]

Bob Stewart wrote:
> Tejun, 
> This is an update to an old thread.
> 
> --- Tejun Heo <htejun@gmail.com> wrote:
>> Bob Stewart wrote:
>>> I wonder if this card of mine is what's bad.  <snipp.
>> My bet is on broken driver.  sunix driver doesn't seem like it was 
>> written carefully.  Anyways, thanks for testing.  It seems we'll either 
>> have to implement adma mode.  I'll mark the driver broken for 2.6.21. 
>> Thanks.
> 
> I just got your note that there is a new SATA_INIC162X driver, so I
> loaded up kernel 2.6.27-rc2 and gave it a try.  Timeouts again, so I
> ordered a new card on ebay.  I'm sad to say that the errors, months
> ago, were from a card that I had gotten bad from new.  Wasted a lot of my
> time and this list's time.  Sorry about that.  :(
> 
> OTOH, with the new card, both your new driver and the old one seem
> to be OK after several hours of copying ISO files and running "md5sum"
> on the copied filed.  Good job!  Thanks for all the hard work!  If there are
> any other tests needed on this driver let me know before I bow out again.

Ah.. great, so at least something works.  :-)

Any chance you can put the old one under windows and see whether it
works there?

-- 
tejun

Re: Old Thread: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Tejun Heo <htejun@gmail.com> wrote:
> Bob Stewart wrote:
<snip>
> > OTOH, with the new card, both your new driver and the old one seem
> > to be OK after several hours of copying ISO files and running "md5sum"
> > on the copied filed.  Good job!  Thanks for all the hard work!  If there
> are
> > any other tests needed on this driver let me know before I bow out again.
> 
> Ah.. great, so at least something works.  :-)
> 
> Any chance you can put the old one under windows and see whether it
> works there?

Sorry.  It's already gone to "card heaven".  

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Alan Cox]

> both the reading and writing are seriously broken.  I can't tell whether
> they end up in the wrong sectors or garbage is transferred to/from the
> right sectors.

Does this occur regardless of which interface you use (the register
shadow or the DMA command engine ?)

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Jeff Garzik]

Alan Cox wrote:
>> both the reading and writing are seriously broken.  I can't tell whether
>> they end up in the wrong sectors or garbage is transferred to/from the
>> right sectors.
> 
> Does this occur regardless of which interface you use (the register
> shadow or the DMA command engine ?)

I'm curious of this as well.

	Jeff

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Tejun Heo]

Jeff Garzik wrote:
> Alan Cox wrote:
>>> both the reading and writing are seriously broken.  I can't tell whether
>>> they end up in the wrong sectors or garbage is transferred to/from the
>>> right sectors.
>>
>> Does this occur regardless of which interface you use (the register
>> shadow or the DMA command engine ?)
> 
> I'm curious of this as well.

Oh well, I never got around to get the ADMA mode working.  The sunix
driver is TF/quasi-BMDMA based (w/o CONTROL register so I'm pretty sure
they have some problem with LBA48).  The docs[1] I have only have
register description and nothing about the programming model.  Me being
not familiar with ADMA, my try didn't go very far.  IIRC, I couldn't
nudge the controller into the ADMA mode.  Is ADMA some kind of
standardized programming interface?

Ah... another thing to note.  The sata_inic162x uses 0xFF status after
reset before it receives the first D2H Reg FIS from the device thus
making libata believe that there's no device attached to the port.  I
have no idea why they had to use 0xFF for that but they did.  :-(

It seems we'll have to consider 0xFF a valid wait state if SCR is valid
and indicates device presence.  Well, that sounds like a good idea anyway.

Thanks.

-- 
tejun

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Mark Lord]

Tejun Heo wrote:
> IIRC, I couldn't nudge the controller into the ADMA mode.
> Is ADMA some kind of standardized programming interface?

Yes.  Or it was supposed to be.
Just for kicks, try hacking the PCI ID into pdc_adma.c
and see what happens with it there.  That driver does ADMA
for (only) READs & WRITEs, one command at a time.

Cheers

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Alan Cox]

On Fri, 16 Mar 2007 08:23:39 -0400
Mark Lord <liml@rtr.ca> wrote:

> Tejun Heo wrote:
> > IIRC, I couldn't nudge the controller into the ADMA mode.
> > Is ADMA some kind of standardized programming interface?

ADMA is documented in the same place as the SFF PCI interface and you'll
find it on the t13 website if I remember rightly

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Jeff Garzik]

Tejun Heo wrote:
> Oh well, I never got around to get the ADMA mode working.  The sunix
> driver is TF/quasi-BMDMA based (w/o CONTROL register so I'm pretty sure
> they have some problem with LBA48).  The docs[1] I have only have
> register description and nothing about the programming model.  Me being


Does this missing reference "[1]" coincide with the docs list I posted 
yesterday?  Do you have all the docs I listed?

	Jeff

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Oh well, I never got around to get the ADMA mode working.  The sunix
>> driver is TF/quasi-BMDMA based (w/o CONTROL register so I'm pretty sure
>> they have some problem with LBA48).  The docs[1] I have only have
>> register description and nothing about the programming model.  Me being
> 
> Does this missing reference "[1]" coincide with the docs list I posted
> yesterday?  Do you have all the docs I listed?

Yeap, I have...

Initio 1622.pdf and two others.  Those are all similar tho.

-- 
tejun

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Jeff Garzik]

Tejun Heo wrote:
> nudge the controller into the ADMA mode.  Is ADMA some kind of
> standardized programming interface?

Yes.  Search for "ADMA" on the linux-ata.org developer resources page: 
http://linux-ata.org/devel.html

Both PDC and NVIDIA based their stuff off of ADMA.  PDC's is closer to 
the standard, though both are pretty darn close.

	Jeff

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Jeff Garzik]

Tejun Heo wrote:
> Hello,
> 
> Bob Stewart wrote:
>> Is there an easy way to tell whether the command was sent as 
>> LBA48, say, in the driver's bmdma_start function or even in the 
>> hardreset function after the fact?  One of my tests was using a 64 
>> sector limit, with pretty much the same results.  I always test against 
>> the same file, and the results, including the number of timeouts, are 
>> different from test to test.
> 
> The driver is seriously broken regarding LBA48 support.  The timeout
> goes away if max_sectors is decreased to ATA_MAX_SECTORS - 1, doh.  But
> both the reading and writing are seriously broken.  I can't tell whether
> they end up in the wrong sectors or garbage is transferred to/from the
> right sectors.
> 
> I'm really close to marking this device broken or we'll need to
> implement a mechanism to veto LBA48 device from LLD (may be negative
> return from ->dev_config).

AFAICS you just need to use the ADMA-like DMA engine, to achieve LBA48.

It's just a guess that the HOB bit is supported by the Device Control 
shadow register on the INIC chips.  The only actual mention in the 
datasheets is in the ADMA CPB data structure definition.

BTW, all my infos came off their public website, let me know if anybody 
is missing these docs:
INIC1622.PDF
Initio INIC-1620TA2_1622TA2_1623TA2 Databook v1.4.pdf
Initio INIC-1623 Databook v1.3.pdf

Regards,

	Jeff

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Bob Stewart]

--- Tejun Heo <htejun@gmail.com> wrote:

> The driver is seriously broken regarding LBA48 support.  The timeout
> goes away if max_sectors is decreased to ATA_MAX_SECTORS - 1, doh.

Tejun,

I should have commented on this earlier.  I still get occasional timeouts
regardless of the max_sectors settings.  Yes, it's much much lower with
some settings vs others, but they're still there.

I was just messing around and did this.  I still got the occasional timeouts.

#define BOBSEGS 64
static int inic_slave_config(struct scsi_device *sdev)
{
        /* This controller is braindamaged.  dma_boundary is 0xffff
         * like others but it will lock up the whole machine HARD if
         * 65536 byte PRD entry is fed.  Reduce maximum segment size.
         */
        struct ata_port *ap = ata_shost_to_port(sdev->host);
        ata_sas_slave_configure(sdev,ap);
        blk_queue_max_phys_segments(sdev->request_queue, BOBSEGS);
        blk_queue_max_hw_segments(sdev->request_queue, BOBSEGS);
        blk_queue_max_sectors(sdev->request_queue, BOBSEGS);
        blk_queue_max_segment_size(sdev->request_queue, BOBSEGS*512);
        return 0;
}


static struct scsi_host_template inic_sht = {
        .sg_tablesize           = BOBSEGS,
...

static void inic_dev_config(struct ata_port *ap, struct ata_device *dev)
{
        /* inic can only handle upto LBA28 max sectors */
        dev->max_sectors = BOBSEGS;


Sorry this isn't in diff format, but I think you can see what I did,
nonetheless.
Eventually, maybe I'll learn enough about something to be able to contribute.

Bob

Re: sata_inic162x driver for 2.6.19 timeouts etc

[Tejun Heo]

Bob Stewart wrote:
> --- Tejun Heo <htejun@gmail.com> wrote:
> 
>> The driver is seriously broken regarding LBA48 support.  The timeout
>> goes away if max_sectors is decreased to ATA_MAX_SECTORS - 1, doh.
> 
> Tejun,
> 
> I should have commented on this earlier.  I still get occasional timeouts
> regardless of the max_sectors settings.  Yes, it's much much lower with
> some settings vs others, but they're still there.
> 
> I was just messing around and did this.  I still got the occasional timeouts.

I see.  Thanks.  We might really need to use the ADMA mode not the
TF/quasi-BMDMA mode.

-- 
tejun

Re: [PATCH] libata: add 150ms between completion of hardreset and status checking

[Jeff Garzik]

Tejun Heo wrote:
> Follow the old SRST rule and delay 150ms between completion of
> hardreset and status checking.  Debouncing delay should usually cover
> this but debounce duration could be shorter than 150ms under certain
> circumstances.
> 
> Usefulness depends on host controller implementation but it can't hurt
> and serves as a reminder that 2s delay for GoVault should also be
> added here.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
>  drivers/ata/libata-core.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

applied