[patch 2/2] sata_via: Delay on vt6420 when starting ATAPI DMA write
Tejun Heo
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.h...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Robert Hancock
> 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.h...@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. :-)
>
--
Bart Hartgers
2010/1/20 Robert Hancock <hanco...@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.h...@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.h...@gmail.com
Tejun Heo
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
Bart Hartgers
2010/1/20 Tejun Heo <hte...@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.h...@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.h...@gmail.com
Tejun Heo
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
Bart Hartgers
2010/1/20 Tejun Heo <hte...@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.h...@gmail.com
Jeff Garzik
> 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?
Bart Hartgers
> 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?
>
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