LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: Commit 9f10d9ee breaks CD mounting/burning
       [not found] <fqngjf$cjf$1@ger.gmane.org>
@ 2008-03-06 21:35 ` Bartlomiej Zolnierkiewicz
  2008-03-07  0:21   ` walt
  2008-03-07  5:48   ` Borislav Petkov
  0 siblings, 2 replies; 7+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2008-03-06 21:35 UTC (permalink / raw)
  To: walt; +Cc: linux-ide, linux-kernel, Borislav Petkov


On Thursday 06 March 2008, walt wrote:
> Hi Bartolmiej,
> 
> For me, this commit causes the problem it's intended to fix:
> 
> commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Date:   Tue Feb 26 21:50:35 2008 +0100
> 
>      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> 
>      This fixes some hangs caused by not finishing the transfer before ending
>      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> 
> When I mount a CD there is a long delay, and I see this error message:
> 
> hdc: ide_cd_check_ireason: wrong transfer direction!
> cdrom: failed setting lba address space
> hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> ide: failed opcode was: unknown
> hdc: drive not ready for command
> <repeated many times>
> 
> When I revert this commit everything works properly again, including
> CD burning.
> 
> Here is the drive info:
> 
> hdc: TSSTcorpCD/DVDW SH-S182M, ATAPI CD/DVD-ROM drive
> hdc: host max PIO5 wanted PIO255(auto-tune) selected PIO4
> hdc: UDMA/33 mode selected
> hdc: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache
> Uniform CD-ROM driver Revision: 3.20
> hdc: UDMA/33 mode selected
> 
> I'm happy to try patches or supply more info, just let me know
> what you need.

Does the following patch help?

[ Borislav, please take a look and double check that it is OK. ]

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide-cd: mark REQ_TYPE_ATA_PC write requests with REQ_RW flag

On Thursday 06 March 2008, walt wrote:

> For me, this commit causes the problem it's intended to fix:
> 
> commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Date:   Tue Feb 26 21:50:35 2008 +0100
> 
>      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> 
>      This fixes some hangs caused by not finishing the transfer before ending
>      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> 
> When I mount a CD there is a long delay, and I see this error message:
> 
> hdc: ide_cd_check_ireason: wrong transfer direction!
> cdrom: failed setting lba address space
> hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> ide: failed opcode was: unknown
> hdc: drive not ready for command
> <repeated many times>
> 
> When I revert this commit everything works properly again, including
> CD burning.

It turned out that REQ_TYPE_ATA_PC write requests were not marked as such
(the previous commit assumed them to be).

Reported-by: walt <w41ter@gmail.com>
Cc: Borislav Petkov <petkovbb@googlemail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-cd_ioctl.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: b/drivers/ide/ide-cd_ioctl.c
===================================================================
--- a/drivers/ide/ide-cd_ioctl.c
+++ b/drivers/ide/ide-cd_ioctl.c
@@ -457,6 +457,10 @@ int ide_cdrom_packet(struct cdrom_device
 	   layer. the packet must be complete, as we do not
 	   touch it at all. */
 	ide_cd_init_rq(drive, &req);
+
+	if (cgc->data_direction == CGC_DATA_WRITE)
+		req.cmd_flags |= REQ_RW;
+
 	memcpy(req.cmd, cgc->cmd, CDROM_PACKET_SIZE);
 	if (cgc->sense)
 		memset(cgc->sense, 0, sizeof(struct request_sense));


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

* Re: Commit 9f10d9ee breaks CD mounting/burning
  2008-03-06 21:35 ` Commit 9f10d9ee breaks CD mounting/burning Bartlomiej Zolnierkiewicz
@ 2008-03-07  0:21   ` walt
  2008-03-07  5:48   ` Borislav Petkov
  1 sibling, 0 replies; 7+ messages in thread
From: walt @ 2008-03-07  0:21 UTC (permalink / raw)
  To: linux-ide; +Cc: linux-kernel, Borislav Petkov

Bartlomiej Zolnierkiewicz wrote:
> On Thursday 06 March 2008, walt wrote:
>> Hi Bartolmiej,
>>
>> For me, this commit causes the problem it's intended to fix:
>>
>> commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
>> Author: Bartlomiej Zolnierkiewicz<bzolnier@gmail.com>
>> Date:   Tue Feb 26 21:50:35 2008 +0100
>>
>>       ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests


> Does the following patch help?
>
> [ Borislav, please take a look and double check that it is OK. ]
>
> From: Bartlomiej Zolnierkiewicz<bzolnier@gmail.com>
> Subject: [PATCH] ide-cd: mark REQ_TYPE_ATA_PC write requests with REQ_RW flag
>
> It turned out that REQ_TYPE_ATA_PC write requests were not marked as such
> (the previous commit assumed them to be).
>
> Reported-by: walt<w41ter@gmail.com>
> Cc: Borislav Petkov<petkovbb@googlemail.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz<bzolnier@gmail.com>
> ---
>   drivers/ide/ide-cd_ioctl.c |    4 ++++
>   1 file changed, 4 insertions(+)
>
> Index: b/drivers/ide/ide-cd_ioctl.c
> ===================================================================
> --- a/drivers/ide/ide-cd_ioctl.c
> +++ b/drivers/ide/ide-cd_ioctl.c
> @@ -457,6 +457,10 @@ int ide_cdrom_packet(struct cdrom_device
>   	   layer. the packet must be complete, as we do not
>   	   touch it at all. */
>   	ide_cd_init_rq(drive,&req);
> +
> +	if (cgc->data_direction == CGC_DATA_WRITE)
> +		req.cmd_flags |= REQ_RW;
> +
>   	memcpy(req.cmd, cgc->cmd, CDROM_PACKET_SIZE);
>   	if (cgc->sense)
>   		memset(cgc->sense, 0, sizeof(struct request_sense));
>

It's perfect, thanks!

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

* Re: Commit 9f10d9ee breaks CD mounting/burning
  2008-03-06 21:35 ` Commit 9f10d9ee breaks CD mounting/burning Bartlomiej Zolnierkiewicz
  2008-03-07  0:21   ` walt
@ 2008-03-07  5:48   ` Borislav Petkov
  2008-03-07 20:39     ` Bartlomiej Zolnierkiewicz
  1 sibling, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2008-03-07  5:48 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: walt, linux-ide, linux-kernel

On Thu, Mar 06, 2008 at 10:35:00PM +0100, Bartlomiej Zolnierkiewicz wrote:
> 
> On Thursday 06 March 2008, walt wrote:
> > Hi Bartolmiej,
> > 
> > For me, this commit causes the problem it's intended to fix:
> > 
> > commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> > Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > Date:   Tue Feb 26 21:50:35 2008 +0100
> > 
> >      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> > 
> >      This fixes some hangs caused by not finishing the transfer before ending
> >      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> > 
> > When I mount a CD there is a long delay, and I see this error message:
> > 
> > hdc: ide_cd_check_ireason: wrong transfer direction!
> > cdrom: failed setting lba address space
> > hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> > ide: failed opcode was: unknown
> > hdc: drive not ready for command
> > <repeated many times>
> > 
> > When I revert this commit everything works properly again, including
> > CD burning.
> > 
> > Here is the drive info:
> > 
> > hdc: TSSTcorpCD/DVDW SH-S182M, ATAPI CD/DVD-ROM drive
> > hdc: host max PIO5 wanted PIO255(auto-tune) selected PIO4
> > hdc: UDMA/33 mode selected
> > hdc: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache
> > Uniform CD-ROM driver Revision: 3.20
> > hdc: UDMA/33 mode selected
> > 
> > I'm happy to try patches or supply more info, just let me know
> > what you need.
> 
> Does the following patch help?
> 
> [ Borislav, please take a look and double check that it is OK. ]
> 
> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Subject: [PATCH] ide-cd: mark REQ_TYPE_ATA_PC write requests with REQ_RW flag
> 
> On Thursday 06 March 2008, walt wrote:
> 
> > For me, this commit causes the problem it's intended to fix:
> > 
> > commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> > Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > Date:   Tue Feb 26 21:50:35 2008 +0100
> > 
> >      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> > 
> >      This fixes some hangs caused by not finishing the transfer before ending
> >      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> > 
> > When I mount a CD there is a long delay, and I see this error message:
> > 
> > hdc: ide_cd_check_ireason: wrong transfer direction!
> > cdrom: failed setting lba address space
> > hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> > ide: failed opcode was: unknown
> > hdc: drive not ready for command
> > <repeated many times>
> > 
> > When I revert this commit everything works properly again, including
> > CD burning.
> 
> It turned out that REQ_TYPE_ATA_PC write requests were not marked as such
> (the previous commit assumed them to be).
> 
> Reported-by: walt <w41ter@gmail.com>
> Cc: Borislav Petkov <petkovbb@googlemail.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---
>  drivers/ide/ide-cd_ioctl.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> Index: b/drivers/ide/ide-cd_ioctl.c
> ===================================================================
> --- a/drivers/ide/ide-cd_ioctl.c
> +++ b/drivers/ide/ide-cd_ioctl.c
> @@ -457,6 +457,10 @@ int ide_cdrom_packet(struct cdrom_device
>  	   layer. the packet must be complete, as we do not
>  	   touch it at all. */
>  	ide_cd_init_rq(drive, &req);
> +
> +	if (cgc->data_direction == CGC_DATA_WRITE)
> +		req.cmd_flags |= REQ_RW;
> +
>  	memcpy(req.cmd, cgc->cmd, CDROM_PACKET_SIZE);
>  	if (cgc->sense)
>  		memset(cgc->sense, 0, sizeof(struct request_sense));

Here's how i understand it (and correct me if i'm wrong):

req.cmd_flags & 0x1 (i.e. the least sig. bit) was remaining unset for write requests
and that's why the ireason check in the interrupt handler was failing loudly.
This sets it correctly so that rq_data_dir(rq) evaluates to the correct data
direction of the request, no?

-- 
Regards/Gruß,
    Boris.

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

* Re: Commit 9f10d9ee breaks CD mounting/burning
  2008-03-07  5:48   ` Borislav Petkov
@ 2008-03-07 20:39     ` Bartlomiej Zolnierkiewicz
  2008-03-07 21:49       ` Borislav Petkov
  0 siblings, 1 reply; 7+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2008-03-07 20:39 UTC (permalink / raw)
  To: petkovbb; +Cc: walt, linux-ide, linux-kernel

On Friday 07 March 2008, Borislav Petkov wrote:
> On Thu, Mar 06, 2008 at 10:35:00PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > 
> > On Thursday 06 March 2008, walt wrote:
> > > Hi Bartolmiej,
> > > 
> > > For me, this commit causes the problem it's intended to fix:
> > > 
> > > commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> > > Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > Date:   Tue Feb 26 21:50:35 2008 +0100
> > > 
> > >      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> > > 
> > >      This fixes some hangs caused by not finishing the transfer before ending
> > >      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> > > 
> > > When I mount a CD there is a long delay, and I see this error message:
> > > 
> > > hdc: ide_cd_check_ireason: wrong transfer direction!
> > > cdrom: failed setting lba address space
> > > hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> > > ide: failed opcode was: unknown
> > > hdc: drive not ready for command
> > > <repeated many times>
> > > 
> > > When I revert this commit everything works properly again, including
> > > CD burning.
> > > 
> > > Here is the drive info:
> > > 
> > > hdc: TSSTcorpCD/DVDW SH-S182M, ATAPI CD/DVD-ROM drive
> > > hdc: host max PIO5 wanted PIO255(auto-tune) selected PIO4
> > > hdc: UDMA/33 mode selected
> > > hdc: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache
> > > Uniform CD-ROM driver Revision: 3.20
> > > hdc: UDMA/33 mode selected
> > > 
> > > I'm happy to try patches or supply more info, just let me know
> > > what you need.
> > 
> > Does the following patch help?
> > 
> > [ Borislav, please take a look and double check that it is OK. ]
> > 
> > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > Subject: [PATCH] ide-cd: mark REQ_TYPE_ATA_PC write requests with REQ_RW flag
> > 
> > On Thursday 06 March 2008, walt wrote:
> > 
> > > For me, this commit causes the problem it's intended to fix:
> > > 
> > > commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> > > Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > Date:   Tue Feb 26 21:50:35 2008 +0100
> > > 
> > >      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> > > 
> > >      This fixes some hangs caused by not finishing the transfer before ending
> > >      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> > > 
> > > When I mount a CD there is a long delay, and I see this error message:
> > > 
> > > hdc: ide_cd_check_ireason: wrong transfer direction!
> > > cdrom: failed setting lba address space
> > > hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> > > ide: failed opcode was: unknown
> > > hdc: drive not ready for command
> > > <repeated many times>
> > > 
> > > When I revert this commit everything works properly again, including
> > > CD burning.
> > 
> > It turned out that REQ_TYPE_ATA_PC write requests were not marked as such
> > (the previous commit assumed them to be).
> > 
> > Reported-by: walt <w41ter@gmail.com>
> > Cc: Borislav Petkov <petkovbb@googlemail.com>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > ---
> >  drivers/ide/ide-cd_ioctl.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > Index: b/drivers/ide/ide-cd_ioctl.c
> > ===================================================================
> > --- a/drivers/ide/ide-cd_ioctl.c
> > +++ b/drivers/ide/ide-cd_ioctl.c
> > @@ -457,6 +457,10 @@ int ide_cdrom_packet(struct cdrom_device
> >  	   layer. the packet must be complete, as we do not
> >  	   touch it at all. */
> >  	ide_cd_init_rq(drive, &req);
> > +
> > +	if (cgc->data_direction == CGC_DATA_WRITE)
> > +		req.cmd_flags |= REQ_RW;
> > +
> >  	memcpy(req.cmd, cgc->cmd, CDROM_PACKET_SIZE);
> >  	if (cgc->sense)
> >  		memset(cgc->sense, 0, sizeof(struct request_sense));
> 
> Here's how i understand it (and correct me if i'm wrong):
> 
> req.cmd_flags & 0x1 (i.e. the least sig. bit) was remaining unset for write requests
> and that's why the ireason check in the interrupt handler was failing loudly.
> This sets it correctly so that rq_data_dir(rq) evaluates to the correct data
> direction of the request, no?

Yep.

Thanks for checking it, I'll push the patch to Linus.

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

* Re: Commit 9f10d9ee breaks CD mounting/burning
  2008-03-07 20:39     ` Bartlomiej Zolnierkiewicz
@ 2008-03-07 21:49       ` Borislav Petkov
  2008-03-07 22:45         ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2008-03-07 21:49 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: walt, linux-ide, linux-kernel

On Fri, Mar 07, 2008 at 09:39:43PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Friday 07 March 2008, Borislav Petkov wrote:
> > On Thu, Mar 06, 2008 at 10:35:00PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > 
> > > On Thursday 06 March 2008, walt wrote:
> > > > Hi Bartolmiej,
> > > > 
> > > > For me, this commit causes the problem it's intended to fix:
> > > > 
> > > > commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> > > > Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > Date:   Tue Feb 26 21:50:35 2008 +0100
> > > > 
> > > >      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> > > > 
> > > >      This fixes some hangs caused by not finishing the transfer before ending
> > > >      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> > > > 
> > > > When I mount a CD there is a long delay, and I see this error message:
> > > > 
> > > > hdc: ide_cd_check_ireason: wrong transfer direction!
> > > > cdrom: failed setting lba address space
> > > > hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> > > > ide: failed opcode was: unknown
> > > > hdc: drive not ready for command
> > > > <repeated many times>
> > > > 
> > > > When I revert this commit everything works properly again, including
> > > > CD burning.
> > > > 
> > > > Here is the drive info:
> > > > 
> > > > hdc: TSSTcorpCD/DVDW SH-S182M, ATAPI CD/DVD-ROM drive
> > > > hdc: host max PIO5 wanted PIO255(auto-tune) selected PIO4
> > > > hdc: UDMA/33 mode selected
> > > > hdc: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache
> > > > Uniform CD-ROM driver Revision: 3.20
> > > > hdc: UDMA/33 mode selected
> > > > 
> > > > I'm happy to try patches or supply more info, just let me know
> > > > what you need.
> > > 
> > > Does the following patch help?
> > > 
> > > [ Borislav, please take a look and double check that it is OK. ]
> > > 
> > > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > Subject: [PATCH] ide-cd: mark REQ_TYPE_ATA_PC write requests with REQ_RW flag
> > > 
> > > On Thursday 06 March 2008, walt wrote:
> > > 
> > > > For me, this commit causes the problem it's intended to fix:
> > > > 
> > > > commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> > > > Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > Date:   Tue Feb 26 21:50:35 2008 +0100
> > > > 
> > > >      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> > > > 
> > > >      This fixes some hangs caused by not finishing the transfer before ending
> > > >      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> > > > 
> > > > When I mount a CD there is a long delay, and I see this error message:
> > > > 
> > > > hdc: ide_cd_check_ireason: wrong transfer direction!
> > > > cdrom: failed setting lba address space
> > > > hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> > > > ide: failed opcode was: unknown
> > > > hdc: drive not ready for command
> > > > <repeated many times>
> > > > 
> > > > When I revert this commit everything works properly again, including
> > > > CD burning.
> > > 
> > > It turned out that REQ_TYPE_ATA_PC write requests were not marked as such
> > > (the previous commit assumed them to be).
> > > 
> > > Reported-by: walt <w41ter@gmail.com>
> > > Cc: Borislav Petkov <petkovbb@googlemail.com>
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > ---
> > >  drivers/ide/ide-cd_ioctl.c |    4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > Index: b/drivers/ide/ide-cd_ioctl.c
> > > ===================================================================
> > > --- a/drivers/ide/ide-cd_ioctl.c
> > > +++ b/drivers/ide/ide-cd_ioctl.c
> > > @@ -457,6 +457,10 @@ int ide_cdrom_packet(struct cdrom_device
> > >  	   layer. the packet must be complete, as we do not
> > >  	   touch it at all. */
> > >  	ide_cd_init_rq(drive, &req);
> > > +
> > > +	if (cgc->data_direction == CGC_DATA_WRITE)
> > > +		req.cmd_flags |= REQ_RW;
> > > +
> > >  	memcpy(req.cmd, cgc->cmd, CDROM_PACKET_SIZE);
> > >  	if (cgc->sense)
> > >  		memset(cgc->sense, 0, sizeof(struct request_sense));
> > 
> > Here's how i understand it (and correct me if i'm wrong):
> > 
> > req.cmd_flags & 0x1 (i.e. the least sig. bit) was remaining unset for write requests
> > and that's why the ireason check in the interrupt handler was failing loudly.
> > This sets it correctly so that rq_data_dir(rq) evaluates to the correct data
> > direction of the request, no?
> 
> Yep.
> 
> Thanks for checking it, I'll push the patch to Linus.

Cool, can we close http://bugzilla.kernel.org/show_bug.cgi?id=10185 now?

-- 
Regards/Gruß,
    Boris.

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

* Re: Commit 9f10d9ee breaks CD mounting/burning
  2008-03-07 21:49       ` Borislav Petkov
@ 2008-03-07 22:45         ` Bartlomiej Zolnierkiewicz
  2008-03-07 23:35           ` Rafael J. Wysocki
  0 siblings, 1 reply; 7+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2008-03-07 22:45 UTC (permalink / raw)
  To: petkovbb; +Cc: walt, linux-ide, linux-kernel

On Friday 07 March 2008, Borislav Petkov wrote:
> On Fri, Mar 07, 2008 at 09:39:43PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Friday 07 March 2008, Borislav Petkov wrote:
> > > On Thu, Mar 06, 2008 at 10:35:00PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > > 
> > > > On Thursday 06 March 2008, walt wrote:
> > > > > Hi Bartolmiej,
> > > > > 
> > > > > For me, this commit causes the problem it's intended to fix:
> > > > > 
> > > > > commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> > > > > Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > > Date:   Tue Feb 26 21:50:35 2008 +0100
> > > > > 
> > > > >      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> > > > > 
> > > > >      This fixes some hangs caused by not finishing the transfer before ending
> > > > >      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> > > > > 
> > > > > When I mount a CD there is a long delay, and I see this error message:
> > > > > 
> > > > > hdc: ide_cd_check_ireason: wrong transfer direction!
> > > > > cdrom: failed setting lba address space
> > > > > hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> > > > > ide: failed opcode was: unknown
> > > > > hdc: drive not ready for command
> > > > > <repeated many times>
> > > > > 
> > > > > When I revert this commit everything works properly again, including
> > > > > CD burning.
> > > > > 
> > > > > Here is the drive info:
> > > > > 
> > > > > hdc: TSSTcorpCD/DVDW SH-S182M, ATAPI CD/DVD-ROM drive
> > > > > hdc: host max PIO5 wanted PIO255(auto-tune) selected PIO4
> > > > > hdc: UDMA/33 mode selected
> > > > > hdc: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache
> > > > > Uniform CD-ROM driver Revision: 3.20
> > > > > hdc: UDMA/33 mode selected
> > > > > 
> > > > > I'm happy to try patches or supply more info, just let me know
> > > > > what you need.
> > > > 
> > > > Does the following patch help?
> > > > 
> > > > [ Borislav, please take a look and double check that it is OK. ]
> > > > 
> > > > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > Subject: [PATCH] ide-cd: mark REQ_TYPE_ATA_PC write requests with REQ_RW flag
> > > > 
> > > > On Thursday 06 March 2008, walt wrote:
> > > > 
> > > > > For me, this commit causes the problem it's intended to fix:
> > > > > 
> > > > > commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> > > > > Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > > Date:   Tue Feb 26 21:50:35 2008 +0100
> > > > > 
> > > > >      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> > > > > 
> > > > >      This fixes some hangs caused by not finishing the transfer before ending
> > > > >      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> > > > > 
> > > > > When I mount a CD there is a long delay, and I see this error message:
> > > > > 
> > > > > hdc: ide_cd_check_ireason: wrong transfer direction!
> > > > > cdrom: failed setting lba address space
> > > > > hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> > > > > ide: failed opcode was: unknown
> > > > > hdc: drive not ready for command
> > > > > <repeated many times>
> > > > > 
> > > > > When I revert this commit everything works properly again, including
> > > > > CD burning.
> > > > 
> > > > It turned out that REQ_TYPE_ATA_PC write requests were not marked as such
> > > > (the previous commit assumed them to be).
> > > > 
> > > > Reported-by: walt <w41ter@gmail.com>
> > > > Cc: Borislav Petkov <petkovbb@googlemail.com>
> > > > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > ---
> > > >  drivers/ide/ide-cd_ioctl.c |    4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > Index: b/drivers/ide/ide-cd_ioctl.c
> > > > ===================================================================
> > > > --- a/drivers/ide/ide-cd_ioctl.c
> > > > +++ b/drivers/ide/ide-cd_ioctl.c
> > > > @@ -457,6 +457,10 @@ int ide_cdrom_packet(struct cdrom_device
> > > >  	   layer. the packet must be complete, as we do not
> > > >  	   touch it at all. */
> > > >  	ide_cd_init_rq(drive, &req);
> > > > +
> > > > +	if (cgc->data_direction == CGC_DATA_WRITE)
> > > > +		req.cmd_flags |= REQ_RW;
> > > > +
> > > >  	memcpy(req.cmd, cgc->cmd, CDROM_PACKET_SIZE);
> > > >  	if (cgc->sense)
> > > >  		memset(cgc->sense, 0, sizeof(struct request_sense));
> > > 
> > > Here's how i understand it (and correct me if i'm wrong):
> > > 
> > > req.cmd_flags & 0x1 (i.e. the least sig. bit) was remaining unset for write requests
> > > and that's why the ireason check in the interrupt handler was failing loudly.
> > > This sets it correctly so that rq_data_dir(rq) evaluates to the correct data
> > > direction of the request, no?
> > 
> > Yep.
> > 
> > Thanks for checking it, I'll push the patch to Linus.
> 
> Cool, can we close http://bugzilla.kernel.org/show_bug.cgi?id=10185 now?

I suppose that Rafael would prefer to close it when patch hits the mainline.

Thanks,
Bart

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

* Re: Commit 9f10d9ee breaks CD mounting/burning
  2008-03-07 22:45         ` Bartlomiej Zolnierkiewicz
@ 2008-03-07 23:35           ` Rafael J. Wysocki
  0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2008-03-07 23:35 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: petkovbb, walt, linux-ide, linux-kernel

On Friday, 7 of March 2008, Bartlomiej Zolnierkiewicz wrote:
> On Friday 07 March 2008, Borislav Petkov wrote:
> > On Fri, Mar 07, 2008 at 09:39:43PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > On Friday 07 March 2008, Borislav Petkov wrote:
> > > > On Thu, Mar 06, 2008 at 10:35:00PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > > > 
> > > > > On Thursday 06 March 2008, walt wrote:
> > > > > > Hi Bartolmiej,
> > > > > > 
> > > > > > For me, this commit causes the problem it's intended to fix:
> > > > > > 
> > > > > > commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> > > > > > Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > > > Date:   Tue Feb 26 21:50:35 2008 +0100
> > > > > > 
> > > > > >      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> > > > > > 
> > > > > >      This fixes some hangs caused by not finishing the transfer before ending
> > > > > >      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> > > > > > 
> > > > > > When I mount a CD there is a long delay, and I see this error message:
> > > > > > 
> > > > > > hdc: ide_cd_check_ireason: wrong transfer direction!
> > > > > > cdrom: failed setting lba address space
> > > > > > hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> > > > > > ide: failed opcode was: unknown
> > > > > > hdc: drive not ready for command
> > > > > > <repeated many times>
> > > > > > 
> > > > > > When I revert this commit everything works properly again, including
> > > > > > CD burning.
> > > > > > 
> > > > > > Here is the drive info:
> > > > > > 
> > > > > > hdc: TSSTcorpCD/DVDW SH-S182M, ATAPI CD/DVD-ROM drive
> > > > > > hdc: host max PIO5 wanted PIO255(auto-tune) selected PIO4
> > > > > > hdc: UDMA/33 mode selected
> > > > > > hdc: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache
> > > > > > Uniform CD-ROM driver Revision: 3.20
> > > > > > hdc: UDMA/33 mode selected
> > > > > > 
> > > > > > I'm happy to try patches or supply more info, just let me know
> > > > > > what you need.
> > > > > 
> > > > > Does the following patch help?
> > > > > 
> > > > > [ Borislav, please take a look and double check that it is OK. ]
> > > > > 
> > > > > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > > Subject: [PATCH] ide-cd: mark REQ_TYPE_ATA_PC write requests with REQ_RW flag
> > > > > 
> > > > > On Thursday 06 March 2008, walt wrote:
> > > > > 
> > > > > > For me, this commit causes the problem it's intended to fix:
> > > > > > 
> > > > > > commit 9f10d9ee0ac6d79d7bc8b9a158bf4a29322d84d3
> > > > > > Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > > > Date:   Tue Feb 26 21:50:35 2008 +0100
> > > > > > 
> > > > > >      ide-cd: fix 'ireason' handling for REQ_TYPE_ATA_PC requests
> > > > > > 
> > > > > >      This fixes some hangs caused by not finishing the transfer before ending
> > > > > >      the request and also makes use of 'ireason == 1' quirk for spurious IRQs.
> > > > > > 
> > > > > > When I mount a CD there is a long delay, and I see this error message:
> > > > > > 
> > > > > > hdc: ide_cd_check_ireason: wrong transfer direction!
> > > > > > cdrom: failed setting lba address space
> > > > > > hdc: status error: status=0x58 { DriveReady SeekComplete DataRequest }
> > > > > > ide: failed opcode was: unknown
> > > > > > hdc: drive not ready for command
> > > > > > <repeated many times>
> > > > > > 
> > > > > > When I revert this commit everything works properly again, including
> > > > > > CD burning.
> > > > > 
> > > > > It turned out that REQ_TYPE_ATA_PC write requests were not marked as such
> > > > > (the previous commit assumed them to be).
> > > > > 
> > > > > Reported-by: walt <w41ter@gmail.com>
> > > > > Cc: Borislav Petkov <petkovbb@googlemail.com>
> > > > > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > > ---
> > > > >  drivers/ide/ide-cd_ioctl.c |    4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > > 
> > > > > Index: b/drivers/ide/ide-cd_ioctl.c
> > > > > ===================================================================
> > > > > --- a/drivers/ide/ide-cd_ioctl.c
> > > > > +++ b/drivers/ide/ide-cd_ioctl.c
> > > > > @@ -457,6 +457,10 @@ int ide_cdrom_packet(struct cdrom_device
> > > > >  	   layer. the packet must be complete, as we do not
> > > > >  	   touch it at all. */
> > > > >  	ide_cd_init_rq(drive, &req);
> > > > > +
> > > > > +	if (cgc->data_direction == CGC_DATA_WRITE)
> > > > > +		req.cmd_flags |= REQ_RW;
> > > > > +
> > > > >  	memcpy(req.cmd, cgc->cmd, CDROM_PACKET_SIZE);
> > > > >  	if (cgc->sense)
> > > > >  		memset(cgc->sense, 0, sizeof(struct request_sense));
> > > > 
> > > > Here's how i understand it (and correct me if i'm wrong):
> > > > 
> > > > req.cmd_flags & 0x1 (i.e. the least sig. bit) was remaining unset for write requests
> > > > and that's why the ireason check in the interrupt handler was failing loudly.
> > > > This sets it correctly so that rq_data_dir(rq) evaluates to the correct data
> > > > direction of the request, no?
> > > 
> > > Yep.
> > > 
> > > Thanks for checking it, I'll push the patch to Linus.
> > 
> > Cool, can we close http://bugzilla.kernel.org/show_bug.cgi?id=10185 now?
> 
> I suppose that Rafael would prefer to close it when patch hits the mainline.

That's correct.

Thanks,
Rafael

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

end of thread, other threads:[~2008-03-07 23:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <fqngjf$cjf$1@ger.gmane.org>
2008-03-06 21:35 ` Commit 9f10d9ee breaks CD mounting/burning Bartlomiej Zolnierkiewicz
2008-03-07  0:21   ` walt
2008-03-07  5:48   ` Borislav Petkov
2008-03-07 20:39     ` Bartlomiej Zolnierkiewicz
2008-03-07 21:49       ` Borislav Petkov
2008-03-07 22:45         ` Bartlomiej Zolnierkiewicz
2008-03-07 23:35           ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).