All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch 0/2] sata_via: bugfixes for vt6420 and vt6421
@ 2010-01-16 23:56 Bart Hartgers
  2010-01-16 23:56 ` [patch 1/2] sata_via: Correctly setup PIO/DMA for pata slave on vt6421 Bart Hartgers
  2010-01-16 23:56 ` [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write Bart Hartgers
  0 siblings, 2 replies; 17+ messages in thread
From: Bart Hartgers @ 2010-01-16 23:56 UTC (permalink / raw)
  To: linux-kernel, Jeff Garzik; +Cc: linux-ide

Dear all,

The following patches fix two bugs I encountered in the sata_via
driver.

- DMA/PIO timings for the pata slave drive on a vt6421 were not
handled correctly.  

- ATAPI DMA corruption on a vt6420. At least for some burners, the
vt6420 needs a small delay between sending the CDB and starting DMA to
prevent silent data corruption. I found this solution by experimentation
and while it works great for me, it might not work for everyone. Reports
of success or failure are appreciated.

Some more background is included in the individual patches.

Groeten,
Bart
-- 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [patch 1/2] sata_via: Correctly setup PIO/DMA for pata slave on vt6421.
  2010-01-16 23:56 [patch 0/2] sata_via: bugfixes for vt6420 and vt6421 Bart Hartgers
@ 2010-01-16 23:56 ` Bart Hartgers
  2010-01-20  3:33   ` Tejun Heo
  2010-01-20 19:35   ` Jeff Garzik
  2010-01-16 23:56 ` [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write Bart Hartgers
  1 sibling, 2 replies; 17+ messages in thread
From: Bart Hartgers @ 2010-01-16 23:56 UTC (permalink / raw)
  To: linux-kernel, Jeff Garzik; +Cc: linux-ide, Bart Hartgers

[-- Attachment #1: sata_via/vt6421-pata-slave.patch --]
[-- Type: text/plain, Size: 2077 bytes --]

Before only the timings for master were set. Datasheet can be found
here: ftp://ftp.vtbridge.org/Docs/Storage/DS_VT6421A_100_CCPL.PDF
Surprisingly, a slave drive works without this patch. According to the
datasheet, the controller by default derives the DMA mode from the 
Set Features command issued to a drive. Not sure about the PIO
timings, though. The real problem is that the timings for the master
effectively are the ones tuned for the slave. If these support
different UDMA-settings, there is trouble, especially when the slave
supports a higher UDMA than the master. 

Anyhow, using the same mechanism for both master and slave seems like 
a good idea.

Signed-off-by: Bart Hartgers <bart.hartgers@gmail.com>
---
Index: linux-2.6.33-rc4/drivers/ata/sata_via.c
===================================================================
--- linux-2.6.33-rc4.orig/drivers/ata/sata_via.c	2010-01-16 22:28:13.000000000 +0100
+++ linux-2.6.33-rc4/drivers/ata/sata_via.c	2010-01-16 22:29:30.000000000 +0100
@@ -44,7 +44,7 @@
 #include <linux/libata.h>
 
 #define DRV_NAME	"sata_via"
-#define DRV_VERSION	"2.4"
+#define DRV_VERSION	"2.5"
 
 /*
  * vt8251 is different from other sata controllers of VIA.  It has two
@@ -392,14 +392,16 @@ static void vt6421_set_pio_mode(struct a
 {
 	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
 	static const u8 pio_bits[] = { 0xA8, 0x65, 0x65, 0x31, 0x20 };
-	pci_write_config_byte(pdev, PATA_PIO_TIMING, pio_bits[adev->pio_mode - XFER_PIO_0]);
+	pci_write_config_byte(pdev, PATA_PIO_TIMING - adev->devno,
+			      pio_bits[adev->pio_mode - XFER_PIO_0]);
 }
 
 static void vt6421_set_dma_mode(struct ata_port *ap, struct ata_device *adev)
 {
 	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
 	static const u8 udma_bits[] = { 0xEE, 0xE8, 0xE6, 0xE4, 0xE2, 0xE1, 0xE0, 0xE0 };
-	pci_write_config_byte(pdev, PATA_UDMA_TIMING, udma_bits[adev->dma_mode - XFER_UDMA_0]);
+	pci_write_config_byte(pdev, PATA_UDMA_TIMING - adev->devno,
+			      udma_bits[adev->dma_mode - XFER_UDMA_0]);
 }
 
 static const unsigned int svia_bar_sizes[] = {

-- 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write
  2010-01-16 23:56 [patch 0/2] sata_via: bugfixes for vt6420 and vt6421 Bart Hartgers
  2010-01-16 23:56 ` [patch 1/2] sata_via: Correctly setup PIO/DMA for pata slave on vt6421 Bart Hartgers
@ 2010-01-16 23:56 ` Bart Hartgers
  2010-01-20  3:30   ` Tejun Heo
  1 sibling, 1 reply; 17+ messages in thread
From: Bart Hartgers @ 2010-01-16 23:56 UTC (permalink / raw)
  To: linux-kernel, Jeff Garzik; +Cc: linux-ide, Bart Hartgers

[-- Attachment #1: sata_via/vt6420-atapi-dma.patch --]
[-- Type: text/plain, Size: 2478 bytes --]

When writing a disc on certain lite-on dvd-writers (also rebadged as
optiarc/LG/...) connected to a vt6420, the ATAPI CDB ends up in the
datastream and on the disc, causing silent corruption.  Delaying
between sending the CDB and starting DMA seems to prevent this.

I do not know if there are burners that do not suffer from this, but
the patch should be safe for those as well.

There are many reports of this issue, but AFAICT no solution was 
found before. For example:
http://lkml.indiana.edu/hypermail/linux/kernel/0802.3/0561.html

Signed-off-by: Bart Hartgers <bart.hartgers@gmail.com>
---
Index: linux-2.6.33-rc4/drivers/ata/sata_via.c
===================================================================
--- linux-2.6.33-rc4.orig/drivers/ata/sata_via.c	2010-01-16 22:29:30.000000000 +0100
+++ linux-2.6.33-rc4/drivers/ata/sata_via.c	2010-01-16 22:29:38.000000000 +0100
@@ -40,6 +40,8 @@
 #include <linux/blkdev.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <scsi/scsi.h>
+#include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_host.h>
 #include <linux/libata.h>
 
@@ -80,6 +82,7 @@ static int vt8251_scr_write(struct ata_l
 static void svia_tf_load(struct ata_port *ap, const struct ata_taskfile *tf);
 static void svia_noop_freeze(struct ata_port *ap);
 static int vt6420_prereset(struct ata_link *link, unsigned long deadline);
+static void vt6420_bmdma_start(struct ata_queued_cmd *qc);
 static int vt6421_pata_cable_detect(struct ata_port *ap);
 static void vt6421_set_pio_mode(struct ata_port *ap, struct ata_device *adev);
 static void vt6421_set_dma_mode(struct ata_port *ap, struct ata_device *adev);
@@ -121,6 +124,7 @@ static struct ata_port_operations vt6420
 	.inherits		= &svia_base_ops,
 	.freeze			= svia_noop_freeze,
 	.prereset		= vt6420_prereset,
+	.bmdma_start		= vt6420_bmdma_start,
 };
 
 static struct ata_port_operations vt6421_pata_ops = {
@@ -377,6 +381,19 @@ static int vt6420_prereset(struct ata_li
 	return 0;
 }
 
+static void vt6420_bmdma_start(struct ata_queued_cmd *qc)
+{
+	struct ata_port *ap = qc->ap;
+	if ((qc->tf.command == ATA_CMD_PACKET) &&
+	    unlikely(qc->scsicmd->sc_data_direction == DMA_TO_DEVICE)) {
+		/* Prevents corruption on some ATAPI burners */
+		printk_once(KERN_WARNING DRV_NAME
+			    ": fixing DMA to device for ATAPI\n");
+		ata_sff_pause(ap);
+	}
+	ata_bmdma_start(qc);
+}
+
 static int vt6421_pata_cable_detect(struct ata_port *ap)
 {
 	struct pci_dev *pdev = to_pci_dev(ap->host->dev);

-- 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write
  2010-01-16 23:56 ` [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write Bart Hartgers
@ 2010-01-20  3:30   ` Tejun Heo
  2010-01-20  4:53     ` Robert Hancock
  2010-01-20  6:44       ` Bart Hartgers
  0 siblings, 2 replies; 17+ messages in thread
From: Tejun Heo @ 2010-01-20  3:30 UTC (permalink / raw)
  To: Bart Hartgers
  Cc: linux-kernel, Jeff Garzik, linux-ide, juergen.metzdorf,
	markpschool, sporadic.crash, apopov, david, jay4mail

Hello,

On 01/17/2010 08:56 AM, Bart Hartgers wrote:
> When writing a disc on certain lite-on dvd-writers (also rebadged as
> optiarc/LG/...) connected to a vt6420, the ATAPI CDB ends up in the
> datastream and on the disc, causing silent corruption.  Delaying
> between sending the CDB and starting DMA seems to prevent this.
> 
> I do not know if there are burners that do not suffer from this, but
> the patch should be safe for those as well.
> 
> There are many reports of this issue, but AFAICT no solution was 
> found before. For example:
> http://lkml.indiana.edu/hypermail/linux/kernel/0802.3/0561.html
> 
> Signed-off-by: Bart Hartgers <bart.hartgers@gmail.com>

Ah... you found solution for this?  That's great.  This is one of the
three problems that have been lingering for years - the other two
being pata_ali ATAPI DMA problem and sata_sil data corruption problem.
I'll be ecstatic if this fix works.  Just one thing, I don't think
we'll need a warning message there.  It's useful during development
but it doesn't really provide any useful information afterwards.

Digging up the mailing list and cc'ing people who have reported this
problem.  If you still have the affected systems, can you guys please
test the patch in the following message and see whether it fixes the
problem?

  http://article.gmane.org/gmane.linux.kernel/939112/raw

Thanks a lot.  :-)

-- 
tejun

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 1/2] sata_via: Correctly setup PIO/DMA for pata slave on vt6421.
  2010-01-16 23:56 ` [patch 1/2] sata_via: Correctly setup PIO/DMA for pata slave on vt6421 Bart Hartgers
@ 2010-01-20  3:33   ` Tejun Heo
  2010-01-20 19:35   ` Jeff Garzik
  1 sibling, 0 replies; 17+ messages in thread
From: Tejun Heo @ 2010-01-20  3:33 UTC (permalink / raw)
  To: Bart Hartgers; +Cc: linux-kernel, Jeff Garzik, linux-ide

On 01/17/2010 08:56 AM, Bart Hartgers wrote:
> Before only the timings for master were set. Datasheet can be found
> here: ftp://ftp.vtbridge.org/Docs/Storage/DS_VT6421A_100_CCPL.PDF
> Surprisingly, a slave drive works without this patch. According to the
> datasheet, the controller by default derives the DMA mode from the 
> Set Features command issued to a drive. Not sure about the PIO
> timings, though. The real problem is that the timings for the master
> effectively are the ones tuned for the slave. If these support
> different UDMA-settings, there is trouble, especially when the slave
> supports a higher UDMA than the master. 
> 
> Anyhow, using the same mechanism for both master and slave seems like 
> a good idea.
> 
> Signed-off-by: Bart Hartgers <bart.hartgers@gmail.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write
  2010-01-20  3:30   ` Tejun Heo
@ 2010-01-20  4:53     ` Robert Hancock
  2010-01-20  6:33         ` Bart Hartgers
  2010-01-20  6:44       ` Bart Hartgers
  1 sibling, 1 reply; 17+ messages in thread
From: Robert Hancock @ 2010-01-20  4:53 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Bart Hartgers, linux-kernel, Jeff Garzik, linux-ide,
	juergen.metzdorf, markpschool, sporadic.crash, apopov, david,
	jay4mail

On 01/19/2010 09:30 PM, Tejun Heo wrote:
> Hello,
>
> On 01/17/2010 08:56 AM, Bart Hartgers wrote:
>> When writing a disc on certain lite-on dvd-writers (also rebadged as
>> optiarc/LG/...) connected to a vt6420, the ATAPI CDB ends up in the
>> datastream and on the disc, causing silent corruption.  Delaying
>> between sending the CDB and starting DMA seems to prevent this.
>>
>> I do not know if there are burners that do not suffer from this, but
>> the patch should be safe for those as well.
>>
>> There are many reports of this issue, but AFAICT no solution was
>> found before. For example:
>> http://lkml.indiana.edu/hypermail/linux/kernel/0802.3/0561.html
>>
>> Signed-off-by: Bart Hartgers<bart.hartgers@gmail.com>
>
> Ah... you found solution for this?  That's great.  This is one of the
> three problems that have been lingering for years - the other two
> being pata_ali ATAPI DMA problem and sata_sil data corruption problem.
> I'll be ecstatic if this fix works.  Just one thing, I don't think
> we'll need a warning message there.  It's useful during development
> but it doesn't really provide any useful information afterwards.

Another tiny nitpick about the patch, the unlikely() around the 
DMA_TO_DEVICE check probably shouldn't be there - unlikely() is for 
things that will always be either highly unlikely or a slow path, 
neither of which really apply.

>
> Digging up the mailing list and cc'ing people who have reported this
> problem.  If you still have the affected systems, can you guys please
> test the patch in the following message and see whether it fixes the
> problem?
>
>    http://article.gmane.org/gmane.linux.kernel/939112/raw
>
> Thanks a lot.  :-)
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write
  2010-01-20  4:53     ` Robert Hancock
@ 2010-01-20  6:33         ` Bart Hartgers
  0 siblings, 0 replies; 17+ messages in thread
From: Bart Hartgers @ 2010-01-20  6:33 UTC (permalink / raw)
  To: Robert Hancock
  Cc: Tejun Heo, linux-kernel, Jeff Garzik, linux-ide,
	juergen.metzdorf, markpschool, sporadic.crash, apopov, david,
	jay4mail

Hi,

2010/1/20 Robert Hancock <hancockrwd@gmail.com>:
> On 01/19/2010 09:30 PM, Tejun Heo wrote:
>>
>> Hello,
>>
>> On 01/17/2010 08:56 AM, Bart Hartgers wrote:
>>>
>>> When writing a disc on certain lite-on dvd-writers (also rebadged as
>>> optiarc/LG/...) connected to a vt6420, the ATAPI CDB ends up in the
>>> datastream and on the disc, causing silent corruption.  Delaying
>>> between sending the CDB and starting DMA seems to prevent this.
>>>
>>> I do not know if there are burners that do not suffer from this, but
>>> the patch should be safe for those as well.
>>>
>>> There are many reports of this issue, but AFAICT no solution was
>>> found before. For example:
>>> http://lkml.indiana.edu/hypermail/linux/kernel/0802.3/0561.html
>>>
>>> Signed-off-by: Bart Hartgers<bart.hartgers@gmail.com>
>>
>> Ah... you found solution for this?  That's great.  This is one of the
>> three problems that have been lingering for years - the other two
>> being pata_ali ATAPI DMA problem and sata_sil data corruption problem.
>> I'll be ecstatic if this fix works.  Just one thing, I don't think
>> we'll need a warning message there.  It's useful during development
>> but it doesn't really provide any useful information afterwards.
>
> Another tiny nitpick about the patch, the unlikely() around the
> DMA_TO_DEVICE check probably shouldn't be there - unlikely() is for things
> that will always be either highly unlikely or a slow path, neither of which
> really apply.
>

Well, the ata_sff_pause actually does a ndelay(400), and with today's
processors that would be a 'slow path', right?

Groeten,
Bart

>>
>> Digging up the mailing list and cc'ing people who have reported this
>> problem.  If you still have the affected systems, can you guys please
>> test the patch in the following message and see whether it fixes the
>> problem?
>>
>>   http://article.gmane.org/gmane.linux.kernel/939112/raw
>>
>> Thanks a lot.  :-)
>>
>
>



-- 
Bart Hartgers - New e-mail: bart.hartgers@gmail.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA  write
@ 2010-01-20  6:33         ` Bart Hartgers
  0 siblings, 0 replies; 17+ messages in thread
From: Bart Hartgers @ 2010-01-20  6:33 UTC (permalink / raw)
  To: Robert Hancock
  Cc: Tejun Heo, linux-kernel, Jeff Garzik, linux-ide,
	juergen.metzdorf, markpschool, sporadic.crash, apopov, david,
	jay4mail

Hi,

2010/1/20 Robert Hancock <hancockrwd@gmail.com>:
> On 01/19/2010 09:30 PM, Tejun Heo wrote:
>>
>> Hello,
>>
>> On 01/17/2010 08:56 AM, Bart Hartgers wrote:
>>>
>>> When writing a disc on certain lite-on dvd-writers (also rebadged as
>>> optiarc/LG/...) connected to a vt6420, the ATAPI CDB ends up in the
>>> datastream and on the disc, causing silent corruption.  Delaying
>>> between sending the CDB and starting DMA seems to prevent this.
>>>
>>> I do not know if there are burners that do not suffer from this, but
>>> the patch should be safe for those as well.
>>>
>>> There are many reports of this issue, but AFAICT no solution was
>>> found before. For example:
>>> http://lkml.indiana.edu/hypermail/linux/kernel/0802.3/0561.html
>>>
>>> Signed-off-by: Bart Hartgers<bart.hartgers@gmail.com>
>>
>> Ah... you found solution for this?  That's great.  This is one of the
>> three problems that have been lingering for years - the other two
>> being pata_ali ATAPI DMA problem and sata_sil data corruption problem.
>> I'll be ecstatic if this fix works.  Just one thing, I don't think
>> we'll need a warning message there.  It's useful during development
>> but it doesn't really provide any useful information afterwards.
>
> Another tiny nitpick about the patch, the unlikely() around the
> DMA_TO_DEVICE check probably shouldn't be there - unlikely() is for things
> that will always be either highly unlikely or a slow path, neither of which
> really apply.
>

Well, the ata_sff_pause actually does a ndelay(400), and with today's
processors that would be a 'slow path', right?

Groeten,
Bart

>>
>> Digging up the mailing list and cc'ing people who have reported this
>> problem.  If you still have the affected systems, can you guys please
>> test the patch in the following message and see whether it fixes the
>> problem?
>>
>>   http://article.gmane.org/gmane.linux.kernel/939112/raw
>>
>> Thanks a lot.  :-)
>>
>
>



-- 
Bart Hartgers - New e-mail: bart.hartgers@gmail.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write
  2010-01-20  3:30   ` Tejun Heo
@ 2010-01-20  6:44       ` Bart Hartgers
  2010-01-20  6:44       ` Bart Hartgers
  1 sibling, 0 replies; 17+ messages in thread
From: Bart Hartgers @ 2010-01-20  6:44 UTC (permalink / raw)
  To: Tejun Heo
  Cc: linux-kernel, Jeff Garzik, linux-ide, juergen.metzdorf,
	markpschool, sporadic.crash, apopov, david, jay4mail

Hi,

2010/1/20 Tejun Heo <htejun@gmail.com>:
> Hello,
>
> On 01/17/2010 08:56 AM, Bart Hartgers wrote:
>> When writing a disc on certain lite-on dvd-writers (also rebadged as
>> optiarc/LG/...) connected to a vt6420, the ATAPI CDB ends up in the
>> datastream and on the disc, causing silent corruption.  Delaying
>> between sending the CDB and starting DMA seems to prevent this.
>>
>> I do not know if there are burners that do not suffer from this, but
>> the patch should be safe for those as well.
>>
>> There are many reports of this issue, but AFAICT no solution was
>> found before. For example:
>> http://lkml.indiana.edu/hypermail/linux/kernel/0802.3/0561.html
>>
>> Signed-off-by: Bart Hartgers <bart.hartgers@gmail.com>
>
> Ah... you found solution for this?  That's great.  This is one of the
> three problems that have been lingering for years - the other two
> being pata_ali ATAPI DMA problem and sata_sil data corruption problem.
> I'll be ecstatic if this fix works.  Just one thing, I don't think
> we'll need a warning message there.  It's useful during development
> but it doesn't really provide any useful information afterwards.
>

Yes, you're right. I'll drop the printk_once and send another patch
for inclusion. However, for testing I found it very useful to make
sure that I got the right module loaded. So I figured it could be
helpful for the interpretation of success/failure reports.

Assuming that this patch works for other people as well, what is
prefered: resending both patches or just to make a new #2/2 (the
vt6420 one)?

Groeten,
Bart

> Digging up the mailing list and cc'ing people who have reported this
> problem.  If you still have the affected systems, can you guys please
> test the patch in the following message and see whether it fixes the
> problem?
>
>  http://article.gmane.org/gmane.linux.kernel/939112/raw
>
> Thanks a lot.  :-)
>
> --
> tejun
>



-- 
Bart Hartgers - New e-mail: bart.hartgers@gmail.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA  write
@ 2010-01-20  6:44       ` Bart Hartgers
  0 siblings, 0 replies; 17+ messages in thread
From: Bart Hartgers @ 2010-01-20  6:44 UTC (permalink / raw)
  To: Tejun Heo
  Cc: linux-kernel, Jeff Garzik, linux-ide, juergen.metzdorf,
	markpschool, sporadic.crash, apopov, david, jay4mail

Hi,

2010/1/20 Tejun Heo <htejun@gmail.com>:
> Hello,
>
> On 01/17/2010 08:56 AM, Bart Hartgers wrote:
>> When writing a disc on certain lite-on dvd-writers (also rebadged as
>> optiarc/LG/...) connected to a vt6420, the ATAPI CDB ends up in the
>> datastream and on the disc, causing silent corruption.  Delaying
>> between sending the CDB and starting DMA seems to prevent this.
>>
>> I do not know if there are burners that do not suffer from this, but
>> the patch should be safe for those as well.
>>
>> There are many reports of this issue, but AFAICT no solution was
>> found before. For example:
>> http://lkml.indiana.edu/hypermail/linux/kernel/0802.3/0561.html
>>
>> Signed-off-by: Bart Hartgers <bart.hartgers@gmail.com>
>
> Ah... you found solution for this?  That's great.  This is one of the
> three problems that have been lingering for years - the other two
> being pata_ali ATAPI DMA problem and sata_sil data corruption problem.
> I'll be ecstatic if this fix works.  Just one thing, I don't think
> we'll need a warning message there.  It's useful during development
> but it doesn't really provide any useful information afterwards.
>

Yes, you're right. I'll drop the printk_once and send another patch
for inclusion. However, for testing I found it very useful to make
sure that I got the right module loaded. So I figured it could be
helpful for the interpretation of success/failure reports.

Assuming that this patch works for other people as well, what is
prefered: resending both patches or just to make a new #2/2 (the
vt6420 one)?

Groeten,
Bart

> Digging up the mailing list and cc'ing people who have reported this
> problem.  If you still have the affected systems, can you guys please
> test the patch in the following message and see whether it fixes the
> problem?
>
>  http://article.gmane.org/gmane.linux.kernel/939112/raw
>
> Thanks a lot.  :-)
>
> --
> tejun
>



-- 
Bart Hartgers - New e-mail: bart.hartgers@gmail.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write
  2010-01-20  6:33         ` Bart Hartgers
  (?)
@ 2010-01-20  6:54         ` Tejun Heo
  2010-01-20 10:43             ` Bart Hartgers
  -1 siblings, 1 reply; 17+ messages in thread
From: Tejun Heo @ 2010-01-20  6:54 UTC (permalink / raw)
  To: Bart Hartgers
  Cc: Robert Hancock, linux-kernel, Jeff Garzik, linux-ide,
	juergen.metzdorf, markpschool, sporadic.crash, apopov, david,
	jay4mail

Hello,

On 01/20/2010 03:33 PM, Bart Hartgers wrote:
> Well, the ata_sff_pause actually does a ndelay(400), and with today's
> processors that would be a 'slow path', right?

unlikely() is used to mark very unlikely paths not the slow ones and I
don't really think DMA_TO_DEVICE would qualify as an unlikely path.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write
  2010-01-20  6:44       ` Bart Hartgers
  (?)
@ 2010-01-20  6:55       ` Tejun Heo
  2010-02-13 22:47         ` Jeff Garzik
  -1 siblings, 1 reply; 17+ messages in thread
From: Tejun Heo @ 2010-01-20  6:55 UTC (permalink / raw)
  To: Bart Hartgers
  Cc: linux-kernel, Jeff Garzik, linux-ide, juergen.metzdorf,
	markpschool, sporadic.crash, apopov, david, jay4mail

Hello,

On 01/20/2010 03:44 PM, Bart Hartgers wrote:
> Yes, you're right. I'll drop the printk_once and send another patch
> for inclusion. However, for testing I found it very useful to make
> sure that I got the right module loaded. So I figured it could be
> helpful for the interpretation of success/failure reports.

Oh yeah, for testing, having it there is a good idea.

> Assuming that this patch works for other people as well, what is
> prefered: resending both patches or just to make a new #2/2 (the
> vt6420 one)?

I think just re-doing the second patch should be enough.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write
  2010-01-20  6:54         ` Tejun Heo
@ 2010-01-20 10:43             ` Bart Hartgers
  0 siblings, 0 replies; 17+ messages in thread
From: Bart Hartgers @ 2010-01-20 10:43 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Robert Hancock, linux-kernel, Jeff Garzik, linux-ide,
	juergen.metzdorf, markpschool, sporadic.crash, apopov, david,
	jay4mail

Hi,

2010/1/20 Tejun Heo <htejun@gmail.com>:
> Hello,
>
> On 01/20/2010 03:33 PM, Bart Hartgers wrote:
>> Well, the ata_sff_pause actually does a ndelay(400), and with today's
>> processors that would be a 'slow path', right?
>
> unlikely() is used to mark very unlikely paths not the slow ones and I
> don't really think DMA_TO_DEVICE would qualify as an unlikely path.
>

Ok, I am convinced; I'll drop the unlikely.

Groeten,
Bart

> Thanks.
>
> --
> tejun
>



-- 
Bart Hartgers - New e-mail: bart.hartgers@gmail.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA  write
@ 2010-01-20 10:43             ` Bart Hartgers
  0 siblings, 0 replies; 17+ messages in thread
From: Bart Hartgers @ 2010-01-20 10:43 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Robert Hancock, linux-kernel, Jeff Garzik, linux-ide,
	juergen.metzdorf, markpschool, sporadic.crash, apopov, david,
	jay4mail

Hi,

2010/1/20 Tejun Heo <htejun@gmail.com>:
> Hello,
>
> On 01/20/2010 03:33 PM, Bart Hartgers wrote:
>> Well, the ata_sff_pause actually does a ndelay(400), and with today's
>> processors that would be a 'slow path', right?
>
> unlikely() is used to mark very unlikely paths not the slow ones and I
> don't really think DMA_TO_DEVICE would qualify as an unlikely path.
>

Ok, I am convinced; I'll drop the unlikely.

Groeten,
Bart

> Thanks.
>
> --
> tejun
>



-- 
Bart Hartgers - New e-mail: bart.hartgers@gmail.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 1/2] sata_via: Correctly setup PIO/DMA for pata slave on vt6421.
  2010-01-16 23:56 ` [patch 1/2] sata_via: Correctly setup PIO/DMA for pata slave on vt6421 Bart Hartgers
  2010-01-20  3:33   ` Tejun Heo
@ 2010-01-20 19:35   ` Jeff Garzik
  1 sibling, 0 replies; 17+ messages in thread
From: Jeff Garzik @ 2010-01-20 19:35 UTC (permalink / raw)
  To: Bart Hartgers; +Cc: linux-kernel, linux-ide

On 01/16/2010 06:56 PM, Bart Hartgers wrote:
> Before only the timings for master were set. Datasheet can be found
> here: ftp://ftp.vtbridge.org/Docs/Storage/DS_VT6421A_100_CCPL.PDF
> Surprisingly, a slave drive works without this patch. According to the
> datasheet, the controller by default derives the DMA mode from the
> Set Features command issued to a drive. Not sure about the PIO
> timings, though. The real problem is that the timings for the master
> effectively are the ones tuned for the slave. If these support
> different UDMA-settings, there is trouble, especially when the slave
> supports a higher UDMA than the master.
>
> Anyhow, using the same mechanism for both master and slave seems like
> a good idea.
>
> Signed-off-by: Bart Hartgers<bart.hartgers@gmail.com>

applied



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write
  2010-01-20  6:55       ` Tejun Heo
@ 2010-02-13 22:47         ` Jeff Garzik
  2010-02-14 11:20           ` Bart Hartgers
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Garzik @ 2010-02-13 22:47 UTC (permalink / raw)
  To: Bart Hartgers
  Cc: Tejun Heo, linux-kernel, linux-ide, juergen.metzdorf,
	markpschool, sporadic.crash, apopov, david, jay4mail

On 01/20/2010 01:55 AM, Tejun Heo wrote:
> Hello,
>
> On 01/20/2010 03:44 PM, Bart Hartgers wrote:
>> Yes, you're right. I'll drop the printk_once and send another patch
>> for inclusion. However, for testing I found it very useful to make
>> sure that I got the right module loaded. So I figured it could be
>> helpful for the interpretation of success/failure reports.
>
> Oh yeah, for testing, having it there is a good idea.
>
>> Assuming that this patch works for other people as well, what is
>> prefered: resending both patches or just to make a new #2/2 (the
>> vt6420 one)?
>
> I think just re-doing the second patch should be enough.

Agreed...  ping, Bart?


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write
  2010-02-13 22:47         ` Jeff Garzik
@ 2010-02-14 11:20           ` Bart Hartgers
  0 siblings, 0 replies; 17+ messages in thread
From: Bart Hartgers @ 2010-02-14 11:20 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Tejun Heo, linux-kernel, linux-ide, juergen.metzdorf,
	markpschool, sporadic.crash, apopov, david, jay4mail

Jeff Garzik wrote:
> On 01/20/2010 01:55 AM, Tejun Heo wrote:
>> Hello,
>>
>> On 01/20/2010 03:44 PM, Bart Hartgers wrote:
>>> Yes, you're right. I'll drop the printk_once and send another patch
>>> for inclusion. However, for testing I found it very useful to make
>>> sure that I got the right module loaded. So I figured it could be
>>> helpful for the interpretation of success/failure reports.
>>
>> Oh yeah, for testing, having it there is a good idea.
>>
>>> Assuming that this patch works for other people as well, what is
>>> prefered: resending both patches or just to make a new #2/2 (the
>>> vt6420 one)?
>>
>> I think just re-doing the second patch should be enough.
>
> Agreed...  ping, Bart?
>
Hi Jeff,

I was waiting for some feedback on whether this patch actually solves 
the problem for others as well. Unfortunately, I didn't get any response 
so far, that's why I didn't resend the patch yet. But I guess it doesn't 
make sense to wait any longer, so I'll redo the patch and send it in a 
separate mail in a few minutes.

(I also tried searching for people suffering from this bug in the past 3 
months, and couldn't find any. I _did_ see some recent posts on various 
bug-lists by Tejun asking to test my patch, but again no response.)

Groeten,
Bart

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2010-02-14 11:20 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-16 23:56 [patch 0/2] sata_via: bugfixes for vt6420 and vt6421 Bart Hartgers
2010-01-16 23:56 ` [patch 1/2] sata_via: Correctly setup PIO/DMA for pata slave on vt6421 Bart Hartgers
2010-01-20  3:33   ` Tejun Heo
2010-01-20 19:35   ` Jeff Garzik
2010-01-16 23:56 ` [patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write Bart Hartgers
2010-01-20  3:30   ` Tejun Heo
2010-01-20  4:53     ` Robert Hancock
2010-01-20  6:33       ` Bart Hartgers
2010-01-20  6:33         ` Bart Hartgers
2010-01-20  6:54         ` Tejun Heo
2010-01-20 10:43           ` Bart Hartgers
2010-01-20 10:43             ` Bart Hartgers
2010-01-20  6:44     ` Bart Hartgers
2010-01-20  6:44       ` Bart Hartgers
2010-01-20  6:55       ` Tejun Heo
2010-02-13 22:47         ` Jeff Garzik
2010-02-14 11:20           ` Bart Hartgers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.