public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Packet writing problems
@ 2004-08-16 15:16 Frediano Ziglio
  2004-08-16 18:50 ` Peter Osterlund
  0 siblings, 1 reply; 14+ messages in thread
From: Frediano Ziglio @ 2004-08-16 15:16 UTC (permalink / raw)
  To: petero2, axboe; +Cc: linux-kernel

I'm trying to do packet writing with my new DVD writer.

# cat /proc/ide/hdc/model
PIONEER DVD-RW DVR-107D

dma enabled.
After some tests with 2.4 I decided to switch to 2.6.
I used Fedora Core 2 with a vanilla kernel 2.6.8.1 + patch from
http://w1.894.telia.com/~u89404340/patches/packet/2.6/, udftools with
Petero patch (http://w1.894.telia.com/~u89404340/patches/packet/, same
site and author).

However I get a lot of problems mount/unmounting devices...

DVD+RW
mkudffs /dev/hdc does not works... doing a strace opening /dev/hdc for
read/write open returns EROFS (or similar). I tried with blockdev
--setrw but still same errors... Than I used pktsetup. mkudffs with
packet device (/dev/pktcdvd/test) worked so I mounted it (still /dev/hdc
report readonly so I used /dev/pktcdvd/test). I copied a directory (4
MB) an deleted it. Then I unmounted device and then waited... after a
while DVD led turned off and DVD spin down... umount command still not
exited... I waited 20 minutes but still hangs so I restarted my PC.

DVD-RW
After a lot of tests I realized that I have to use dvd+rw-format with
-force=full cause DVD was not empty. Then I used pktsetup and mkudffs
(with no problem). I tried to mount my DVD but mount (not umount) hangs.
Using ps it seems than mount call wait_to_buffer. After 10/15 minutes I
restarted my PC...

I have also a SCSI CD recorder (Waitec WT4260) and a CDRW formatted with
UDF (using 2.4.23 and an old patch from Petero site) and it works
without problems... Is the problem related to IDE or to DVD ??

I'm not (still :) ? ) a linux guru and I don't know how to debug or what
check should I execute... Should I send my .config ? Stop some daemons ?
Work without X ?

freddy77



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

* Re: Packet writing problems
  2004-08-16 15:16 Packet writing problems Frediano Ziglio
@ 2004-08-16 18:50 ` Peter Osterlund
  2004-08-16 19:09   ` Peter Osterlund
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Osterlund @ 2004-08-16 18:50 UTC (permalink / raw)
  To: Frediano Ziglio; +Cc: axboe, linux-kernel

Frediano Ziglio <freddyz77@tin.it> writes:

> I'm trying to do packet writing with my new DVD writer.
> 
> # cat /proc/ide/hdc/model
> PIONEER DVD-RW DVR-107D
> 
> dma enabled.
> After some tests with 2.4 I decided to switch to 2.6.
> I used Fedora Core 2 with a vanilla kernel 2.6.8.1 + patch from
> http://w1.894.telia.com/~u89404340/patches/packet/2.6/, udftools with
> Petero patch (http://w1.894.telia.com/~u89404340/patches/packet/, same
> site and author).
> 
> However I get a lot of problems mount/unmounting devices...
> 
> DVD+RW
> mkudffs /dev/hdc does not works... doing a strace opening /dev/hdc for
> read/write open returns EROFS (or similar). I tried with blockdev
> --setrw but still same errors...

I see two problems. The first problem is that the Mt Rainier detection
can succeed when it shouldn't, because it forgets to check that the
"GET CONFIGURATION" command returns the MRW feature number. On one of
my drives, the command returns feature 0x2a which is DVD+RW.

This bug has nothing to do with the packet writing driver, and the
patch below fixes it.

Signed-off-by: Peter Osterlund <petero2@telia.com>

---

 linux-petero/drivers/cdrom/cdrom.c |    2 ++
 1 files changed, 2 insertions(+)

diff -puN drivers/cdrom/cdrom.c~mrw-fix drivers/cdrom/cdrom.c
--- linux/drivers/cdrom/cdrom.c~mrw-fix	2004-08-16 20:35:45.332325832 +0200
+++ linux-petero/drivers/cdrom/cdrom.c	2004-08-16 20:38:32.513910368 +0200
@@ -521,6 +521,8 @@ int cdrom_is_mrw(struct cdrom_device_inf
 		return ret;
 
 	mfd = (struct mrw_feature_desc *)&buffer[sizeof(struct feature_header)];
+	if (be16_to_cpu(mfd->feature_code) != CDF_MRW)
+		return 1;
 	*write = mfd->write;
 
 	if ((ret = cdrom_mrw_probe_pc(cdi))) {
_

-- 
Peter Osterlund - petero2@telia.com
http://w1.894.telia.com/~u89404340

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

* Re: Packet writing problems
  2004-08-16 18:50 ` Peter Osterlund
@ 2004-08-16 19:09   ` Peter Osterlund
  2004-08-16 19:51     ` Frediano Ziglio
  2004-08-16 19:55     ` Frediano Ziglio
  0 siblings, 2 replies; 14+ messages in thread
From: Peter Osterlund @ 2004-08-16 19:09 UTC (permalink / raw)
  To: Frediano Ziglio; +Cc: axboe, linux-kernel, Andrew Morton

Peter Osterlund <petero2@telia.com> writes:

> Frediano Ziglio <freddyz77@tin.it> writes:
> 
> > I'm trying to do packet writing with my new DVD writer.
> > 
> > # cat /proc/ide/hdc/model
> > PIONEER DVD-RW DVR-107D
> > 
> > dma enabled.
> > After some tests with 2.4 I decided to switch to 2.6.
> > I used Fedora Core 2 with a vanilla kernel 2.6.8.1 + patch from
> > http://w1.894.telia.com/~u89404340/patches/packet/2.6/, udftools with
> > Petero patch (http://w1.894.telia.com/~u89404340/patches/packet/, same
> > site and author).
> > 
> > However I get a lot of problems mount/unmounting devices...
> > 
> > DVD+RW
> > mkudffs /dev/hdc does not works... doing a strace opening /dev/hdc for
> > read/write open returns EROFS (or similar). I tried with blockdev
> > --setrw but still same errors...
> 
> I see two problems. The first problem is that the Mt Rainier detection
> can succeed when it shouldn't, because it forgets to check that the
> "GET CONFIGURATION" command returns the MRW feature number.

The second problem is in the dvdrw-support patch in the -mm kernel.
(This patch is also included in the patch you are using.)

The problem is that some drives fail the "GET CONFIGURATION" command
when asked to only return 8 bytes. This happens for example on my
drive, which is identified as:

        hdc: HL-DT-ST DVD+RW GCA-4040N, ATAPI CD/DVD-ROM drive

Since the cdrom_mmc3_profile() function already allocates 32 bytes for
the reply buffer, this patch is enough to make the command succeed on
my drive.

Signed-off-by: Peter Osterlund <petero2@telia.com>

---

 linux-petero/drivers/cdrom/cdrom.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN drivers/cdrom/cdrom.c~dvd+rw-get-configuration-fix drivers/cdrom/cdrom.c
--- linux/drivers/cdrom/cdrom.c~dvd+rw-get-configuration-fix	2004-08-16 20:54:49.710353928 +0200
+++ linux-petero/drivers/cdrom/cdrom.c	2004-08-16 20:55:41.514478504 +0200
@@ -834,7 +834,7 @@ static void cdrom_mmc3_profile(struct cd
 	cgc.cmd[0] = GPCMD_GET_CONFIGURATION;
 	cgc.cmd[1] = 0;
 	cgc.cmd[2] = cgc.cmd[3] = 0;		/* Starting Feature Number */
-	cgc.cmd[7] = 0; cgc.cmd [8] = 8;	/* Allocation Length */
+	cgc.cmd[8] = sizeof(buffer);		/* Allocation Length */
 	cgc.quiet = 1;
 
 	if ((ret = cdi->ops->generic_packet(cdi, &cgc))) {
_

-- 
Peter Osterlund - petero2@telia.com
http://w1.894.telia.com/~u89404340

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

* Re: Packet writing problems
  2004-08-16 19:09   ` Peter Osterlund
@ 2004-08-16 19:51     ` Frediano Ziglio
  2004-08-16 19:55     ` Frediano Ziglio
  1 sibling, 0 replies; 14+ messages in thread
From: Frediano Ziglio @ 2004-08-16 19:51 UTC (permalink / raw)
  To: Peter Osterlund; +Cc: axboe, linux-kernel, Andrew Morton

Il lun, 2004-08-16 alle 21:09, Peter Osterlund ha scritto:
> ...

> > > 
> > > DVD+RW
> > > mkudffs /dev/hdc does not works... doing a strace opening /dev/hdc for
> > > read/write open returns EROFS (or similar). I tried with blockdev
> > > --setrw but still same errors...
> > 
> > I see two problems. The first problem is that the Mt Rainier detection
> > can succeed when it shouldn't, because it forgets to check that the
> > "GET CONFIGURATION" command returns the MRW feature number.
> 
> The second problem is in the dvdrw-support patch in the -mm kernel.
> (This patch is also included in the patch you are using.)
> 
> The problem is that some drives fail the "GET CONFIGURATION" command
> when asked to only return 8 bytes. This happens for example on my
> drive, which is identified as:
> 
>         hdc: HL-DT-ST DVD+RW GCA-4040N, ATAPI CD/DVD-ROM drive
> 
> Since the cdrom_mmc3_profile() function already allocates 32 bytes for
> the reply buffer, this patch is enough to make the command succeed on
> my drive.
> 

With these two patch both DVD-RW and DVD+RW works !!! Thanks you very much, it's a dream coming true.
It seems that DVD+RW works better (it's faster) with packet device than with direct writing (mounting /dev/hdc).
When I'll get back to work (on September) I'll test your patch with USB 2 device and another DVD writer.

freddy77



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

* Re: Packet writing problems
  2004-08-16 19:09   ` Peter Osterlund
  2004-08-16 19:51     ` Frediano Ziglio
@ 2004-08-16 19:55     ` Frediano Ziglio
  2004-08-17 19:59       ` Peter Osterlund
  1 sibling, 1 reply; 14+ messages in thread
From: Frediano Ziglio @ 2004-08-16 19:55 UTC (permalink / raw)
  To: Peter Osterlund; +Cc: axboe, linux-kernel, Andrew Morton

Il lun, 2004-08-16 alle 21:09, Peter Osterlund ha scritto:
...
> 
> The second problem is in the dvdrw-support patch in the -mm kernel.
> (This patch is also included in the patch you are using.)
> 
> The problem is that some drives fail the "GET CONFIGURATION" command
> when asked to only return 8 bytes. This happens for example on my
> drive, which is identified as:
> 
>         hdc: HL-DT-ST DVD+RW GCA-4040N, ATAPI CD/DVD-ROM drive
> 
> Since the cdrom_mmc3_profile() function already allocates 32 bytes for
> the reply buffer, this patch is enough to make the command succeed on
> my drive.
> 

I'm forgetting... 

mounting devices it reports that disk was CD-RW and speed was 15
(DVD-RW) and 31 (DVD+RW).

freddy77



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

* Re: Packet writing problems
  2004-08-16 19:55     ` Frediano Ziglio
@ 2004-08-17 19:59       ` Peter Osterlund
  2004-08-17 20:24         ` Peter Osterlund
  2004-08-17 23:31         ` Julien Oster
  0 siblings, 2 replies; 14+ messages in thread
From: Peter Osterlund @ 2004-08-17 19:59 UTC (permalink / raw)
  To: Frediano Ziglio; +Cc: axboe, linux-kernel, Andrew Morton

Frediano Ziglio <freddyz77@tin.it> writes:

> Il lun, 2004-08-16 alle 21:09, Peter Osterlund ha scritto:
> ...
> > 
> > The second problem is in the dvdrw-support patch in the -mm kernel.
> > (This patch is also included in the patch you are using.)
> > 
> > The problem is that some drives fail the "GET CONFIGURATION" command
> > when asked to only return 8 bytes. This happens for example on my
> > drive, which is identified as:
> > 
> >         hdc: HL-DT-ST DVD+RW GCA-4040N, ATAPI CD/DVD-ROM drive
> > 
> > Since the cdrom_mmc3_profile() function already allocates 32 bytes for
> > the reply buffer, this patch is enough to make the command succeed on
> > my drive.
> > 
> 
> I'm forgetting... 
> 
> mounting devices it reports that disk was CD-RW and speed was 15
> (DVD-RW) and 31 (DVD+RW).

That shouldn't cause any real problems, but since it's quite
confusing, here is a patch to fix it.  With this change, both DVD+RW
and DVD-RW media is correctly identified in the kernel log, and DVD
speeds are printed in kB/s.

---

 linux-petero/drivers/block/pktcdvd.c |   34 ++++++++++++++++++++--------------
 linux-petero/include/linux/pktcdvd.h |    4 ++--
 2 files changed, 22 insertions(+), 16 deletions(-)

diff -puN drivers/block/pktcdvd.c~packet-dvd-printk-fix drivers/block/pktcdvd.c
--- linux/drivers/block/pktcdvd.c~packet-dvd-printk-fix	2004-08-16 23:51:52.000000000 +0200
+++ linux-petero/drivers/block/pktcdvd.c	2004-08-17 00:33:47.000000000 +0200
@@ -483,11 +483,6 @@ static int pkt_set_speed(struct pktcdvd_
 	struct request_sense sense;
 	int ret;
 
-	write_speed = write_speed * 177; /* should be 176.4, but CD-RWs rounds down */
-	write_speed = min_t(unsigned, write_speed, MAX_SPEED);
-	read_speed = read_speed * 177;
-	read_speed = min_t(unsigned, read_speed, MAX_SPEED);
-
 	init_cdrom_command(&cgc, NULL, 0, CGC_DATA_NONE);
 	cgc.sense = &sense;
 	cgc.cmd[0] = GPCMD_SET_SPEED;
@@ -602,8 +597,8 @@ static void pkt_iosched_process_queue(st
 			pd->iosched.successive_reads = 0;
 		if (pd->iosched.successive_reads >= HI_SPEED_SWITCH) {
 			if (pd->read_speed == pd->write_speed) {
-				pd->read_speed = 0xff;
-				pkt_set_speed(pd, pd->write_speed, MAX_SPEED);
+				pd->read_speed = MAX_SPEED;
+				pkt_set_speed(pd, pd->write_speed, pd->read_speed);
 			}
 		} else {
 			if (pd->read_speed != pd->write_speed) {
@@ -1632,7 +1627,17 @@ static int pkt_probe_settings(struct pkt
 	if (pkt_good_disc(pd, &di))
 		return -ENXIO;
 
-	printk("pktcdvd: inserted media is CD-R%s\n", di.erasable ? "W" : "");
+	switch (pd->mmc3_profile) {
+		case 0x1a: /* DVD+RW */
+			printk("pktcdvd: inserted media is DVD+RW\n");
+			break;
+		case 0x13: /* DVD-RW */
+			printk("pktcdvd: inserted media is DVD-RW\n");
+			break;
+		default:
+			printk("pktcdvd: inserted media is CD-R%s\n", di.erasable ? "W" : "");
+			break;
+	}
 	pd->type = di.erasable ? PACKET_CDRW : PACKET_CDR;
 
 	track = 1; /* (di.last_track_msb << 8) | di.last_track_lsb; */
@@ -1785,7 +1790,7 @@ static int pkt_get_max_speed(struct pktc
 			offset = 34;
 	}
 
-	*write_speed = ((cap_buf[offset] << 8) | cap_buf[offset + 1]) / 0xb0;
+	*write_speed = (cap_buf[offset] << 8) | cap_buf[offset + 1];
 	return 0;
 }
 
@@ -1917,15 +1922,17 @@ static int pkt_open_write(struct pktcdvd
 	pkt_write_caching(pd, USE_WCACHING);
 
 	if ((ret = pkt_get_max_speed(pd, &write_speed)))
-		write_speed = 16;
+		write_speed = 16 * 177;
 	switch (pd->mmc3_profile) {
 		case 0x13: /* DVD-RW */
 		case 0x1a: /* DVD+RW */
+			DPRINTK("pktcdvd: write speed %ukB/s\n", write_speed);
 			break;
 		default:
 			if ((ret = pkt_media_speed(pd, &media_write_speed)))
 				media_write_speed = 16;
-			write_speed = min(write_speed, media_write_speed);
+			write_speed = min(write_speed, media_write_speed * 177);
+			DPRINTK("pktcdvd: write speed %ux\n", write_speed / 176);
 			break;
 	}
 	read_speed = write_speed;
@@ -1934,7 +1941,6 @@ static int pkt_open_write(struct pktcdvd
 		DPRINTK("pktcdvd: %s couldn't set write speed\n", pd->name);
 		return -EIO;
 	}
-	DPRINTK("pktcdvd: write speed %u\n", write_speed);
 	pd->write_speed = write_speed;
 	pd->read_speed = read_speed;
 
@@ -2310,8 +2316,8 @@ static int pkt_seq_show(struct seq_file 
 	seq_printf(m, "\nMisc:\n");
 	seq_printf(m, "\treference count:\t%d\n", pd->refcnt);
 	seq_printf(m, "\tflags:\t\t\t0x%lx\n", pd->flags);
-	seq_printf(m, "\tread speed:\t\t%ukB/s\n", pd->read_speed * 150);
-	seq_printf(m, "\twrite speed:\t\t%ukB/s\n", pd->write_speed * 150);
+	seq_printf(m, "\tread speed:\t\t%ukB/s\n", pd->read_speed);
+	seq_printf(m, "\twrite speed:\t\t%ukB/s\n", pd->write_speed);
 	seq_printf(m, "\tstart offset:\t\t%lu\n", pd->offset);
 	seq_printf(m, "\tmode page offset:\t%u\n", pd->mode_offset);
 
diff -puN include/linux/pktcdvd.h~packet-dvd-printk-fix include/linux/pktcdvd.h
--- linux/include/linux/pktcdvd.h~packet-dvd-printk-fix	2004-08-17 00:34:42.000000000 +0200
+++ linux-petero/include/linux/pktcdvd.h	2004-08-17 00:35:09.000000000 +0200
@@ -239,8 +239,8 @@ struct pktcdvd_device
 	struct packet_settings	settings;
 	struct packet_stats	stats;
 	int			refcnt;		/* Open count */
-	__u8			write_speed;	/* current write speed */
-	__u8			read_speed;	/* current read speed */
+	int			write_speed;	/* current write speed, kB/s */
+	int			read_speed;	/* current read speed, kB/s */
 	unsigned long		offset;		/* start offset */
 	__u8			mode_offset;	/* 0 / 8 */
 	__u8			type;
_

-- 
Peter Osterlund - petero2@telia.com
http://w1.894.telia.com/~u89404340

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

* Re: Packet writing problems
  2004-08-17 19:59       ` Peter Osterlund
@ 2004-08-17 20:24         ` Peter Osterlund
  2004-08-17 23:31         ` Julien Oster
  1 sibling, 0 replies; 14+ messages in thread
From: Peter Osterlund @ 2004-08-17 20:24 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Frediano Ziglio, axboe, linux-kernel

Peter Osterlund <petero2@telia.com> writes:

> Frediano Ziglio <freddyz77@tin.it> writes:
> 
> > Il lun, 2004-08-16 alle 21:09, Peter Osterlund ha scritto:
> > ...
> > > 
> > > The second problem is in the dvdrw-support patch in the -mm kernel.
> > > (This patch is also included in the patch you are using.)
> > > 
> > > The problem is that some drives fail the "GET CONFIGURATION" command
> > > when asked to only return 8 bytes. This happens for example on my
> > > drive, which is identified as:
> > > 
> > >         hdc: HL-DT-ST DVD+RW GCA-4040N, ATAPI CD/DVD-ROM drive
> > > 
> > > Since the cdrom_mmc3_profile() function already allocates 32 bytes for
> > > the reply buffer, this patch is enough to make the command succeed on
> > > my drive.
> > > 
> > 
> > I'm forgetting... 
> > 
> > mounting devices it reports that disk was CD-RW and speed was 15
> > (DVD-RW) and 31 (DVD+RW).
> 
> That shouldn't cause any real problems, but since it's quite
> confusing, here is a patch to fix it.  With this change, both DVD+RW
> and DVD-RW media is correctly identified in the kernel log, and DVD
> speeds are printed in kB/s.

I forgot to say:

Signed-off-by: Peter Osterlund <petero2@telia.com>

-- 
Peter Osterlund - petero2@telia.com
http://w1.894.telia.com/~u89404340

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

* Re: Packet writing problems
  2004-08-17 19:59       ` Peter Osterlund
  2004-08-17 20:24         ` Peter Osterlund
@ 2004-08-17 23:31         ` Julien Oster
  2004-08-17 23:36           ` Peter Osterlund
  1 sibling, 1 reply; 14+ messages in thread
From: Julien Oster @ 2004-08-17 23:31 UTC (permalink / raw)
  To: Peter Osterlund; +Cc: Frediano Ziglio, axboe, linux-kernel, Andrew Morton

Peter Osterlund <petero2@telia.com> writes:

Hello Peter,

> That shouldn't cause any real problems, but since it's quite
> confusing, here is a patch to fix it.  With this change, both DVD+RW
> and DVD-RW media is correctly identified in the kernel log, and DVD
> speeds are printed in kB/s.

The following patch on top of your patch adds all commonly used media
types to the output and changes CD-R and CD-RW to be detected by
profile type. It also reports unconforming non-standard profiles as
well as profiles which have a MMC profile definition but are unknown
as of the current MMC3 revision.

Please review.

--- fuzzy-2.6.8.1-orig/drivers/block/pktcdvd.c	2004-08-18 01:22:53.540198893 +0200
+++ fuzzy-2.6.8.1/drivers/block/pktcdvd.c	2004-08-18 01:24:25.983297748 +0200
@@ -1628,14 +1628,38 @@ static int pkt_probe_settings(struct pkt
 		return -ENXIO;
 
 	switch (pd->mmc3_profile) {
+	        case 0x08: /* CD-ROM */
+	                printk("pktcdvd: inserted media is CD-ROM\n");
+			break;
+	        case 0x09: /* CD-R */
+	                printk("pktcdvd: inserted media is CD-R\n");
+			break;
+	        case 0x0a: /* CD-RW */
+	                printk("pktcdvd: inserted media is CD-RW\n");
+			break;
+	        case 0x10: /* DVD-ROM */
+	                printk("pktcdvd: inserted media is DVD-ROM\n");
+			break;
+	        case 0x11: /* DVD-R */
+	                printk("pktcdvd: inserted media is DVD-R\n");
+			break;
+	        case 0x12: /* DVD-RAM */
+	                printk("pktcdvd: inserted media is DVD-RAM\n");
+			break;
+		case 0x13: /* DVD-RW restricted overwrite */
+			printk("pktcdvd: inserted media is DVD-RW with restricted overwrite\n");
+			break;
+		case 0x14: /* DVD-RW sequential recording */
+			printk("pktcdvd: inserted media is DVD-RW with sequential recording\n");
+			break;
 		case 0x1a: /* DVD+RW */
 			printk("pktcdvd: inserted media is DVD+RW\n");
 			break;
-		case 0x13: /* DVD-RW */
-			printk("pktcdvd: inserted media is DVD-RW\n");
+		case 0xffff: /* unconforming */
+			printk("pktcdvd: inserted media does not conform to a known standard\n");
 			break;
 		default:
-			printk("pktcdvd: inserted media is CD-R%s\n", di.erasable ? "W" : "");
+			printk("pktcdvd: inserted media is yet UNKNOWN by pktcdvd\n");
 			break;
 	}
 	pd->type = di.erasable ? PACKET_CDRW : PACKET_CDR;


Regards,
Julien

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

* Re: Packet writing problems
  2004-08-17 23:31         ` Julien Oster
@ 2004-08-17 23:36           ` Peter Osterlund
  2004-08-18  8:25             ` Julien Oster
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Osterlund @ 2004-08-17 23:36 UTC (permalink / raw)
  To: Julien Oster; +Cc: Frediano Ziglio, axboe, linux-kernel, Andrew Morton

Julien Oster <lkml-7994@mc.frodoid.org> writes:

> Peter Osterlund <petero2@telia.com> writes:
> 
> Hello Peter,
> 
> > That shouldn't cause any real problems, but since it's quite
> > confusing, here is a patch to fix it.  With this change, both DVD+RW
> > and DVD-RW media is correctly identified in the kernel log, and DVD
> > speeds are printed in kB/s.
> 
> The following patch on top of your patch adds all commonly used media
> types to the output and changes CD-R and CD-RW to be detected by
> profile type. It also reports unconforming non-standard profiles as
> well as profiles which have a MMC profile definition but are unknown
> as of the current MMC3 revision.
> 
> Please review.

Will any of those printk's ever get printed? Media types that can't be
handled by the packet driver aren't supposed to make it past the
pkt_good_disc() test.

-- 
Peter Osterlund - petero2@telia.com
http://w1.894.telia.com/~u89404340

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

* Re: Packet writing problems
  2004-08-17 23:36           ` Peter Osterlund
@ 2004-08-18  8:25             ` Julien Oster
  2004-08-18  8:38               ` Julien Oster
  0 siblings, 1 reply; 14+ messages in thread
From: Julien Oster @ 2004-08-18  8:25 UTC (permalink / raw)
  To: Peter Osterlund
  Cc: Julien Oster, Frediano Ziglio, axboe, linux-kernel, Andrew Morton

Peter Osterlund <petero2@telia.com> writes:

Hello Peter,

>> The following patch on top of your patch adds all commonly used media
>> types to the output and changes CD-R and CD-RW to be detected by
>> profile type. It also reports unconforming non-standard profiles as
>> well as profiles which have a MMC profile definition but are unknown
>> as of the current MMC3 revision.

> Will any of those printk's ever get printed? Media types that can't be
> handled by the packet driver aren't supposed to make it past the
> pkt_good_disc() test.

Quickly looking through it, pkt_good_disc() does the same and drops
media types with unsuitable or invalid profiles.

So my patch is really not useful at that place.

But couldn't you move the "inserted disc is..." messages to
pkt_good_disc()? In the current source you basically duplicate the
switch statement which checks the profile. The printks could be in the
pkt_good_disc() check just as well.

There all profiles might be included and the corresponding media type
printed out, so that the user knows why his disc is unsuitable
(i.e. it's a CD-ROM or DVD-ROM and doesn't do any writing at all).

Regards,
Julien

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

* Re: Packet writing problems
  2004-08-18  8:25             ` Julien Oster
@ 2004-08-18  8:38               ` Julien Oster
  2004-08-18  8:48                 ` Julien Oster
  0 siblings, 1 reply; 14+ messages in thread
From: Julien Oster @ 2004-08-18  8:38 UTC (permalink / raw)
  To: Peter Osterlund
  Cc: Julien Oster, Frediano Ziglio, axboe, linux-kernel, Andrew Morton

Julien Oster <usenet-20040502@usenet.frodoid.org> writes:

Hello,

> But couldn't you move the "inserted disc is..." messages to
> pkt_good_disc()? In the current source you basically duplicate the
> switch statement which checks the profile. The printks could be in the
> pkt_good_disc() check just as well.

So here's another patch instead which does what I meant.

Additionally, it inverts the return values of pkt_good_disc, so that
the condition later becomes "if (!pkt_good_disc(...))" (because
actually, it is NOT a good disc then)

--- fuzzy-2.6.8.1-orig/drivers/block/pktcdvd.c	2004-08-18 01:22:53.000000000 +0200
+++ fuzzy-2.6.8.1/drivers/block/pktcdvd.c	2004-08-18 10:34:52.239233284 +0200
@@ -1564,15 +1564,40 @@ static int pkt_good_track(track_informat
 static int pkt_good_disc(struct pktcdvd_device *pd, disc_information *di)
 {
 	switch (pd->mmc3_profile) {
-		case 0x0a: /* CD-RW */
-		case 0xffff: /* MMC3 not supported */
-			break;
-		case 0x1a: /* DVD+RW */
-		case 0x13: /* DVD-RW */
+	        case 0x08: /* CD-ROM */
+	                printk("pktcdvd: inserted media is CD-ROM - no packet writing\n");
 			return 0;
-		default:
-			printk("pktcdvd: Wrong disc profile (%x)\n", pd->mmc3_profile);
+	        case 0x09: /* CD-R */
+	                printk("pktcdvd: inserted media is CD-R - no packet writing\n");
+			return 0;
+	        case 0x0a: /* CD-RW */
+	                printk("pktcdvd: inserted media is CD-RW\n");
+			return 1;
+	        case 0x10: /* DVD-ROM */
+	                printk("pktcdvd: inserted media is DVD-ROM - no packet writing\n");
+			return 0;
+	        case 0x11: /* DVD-R */
+	                printk("pktcdvd: inserted media is DVD-R - no packet writing\n");
+			return 0;
+	        case 0x12: /* DVD-RAM */
+	                printk("pktcdvd: inserted media is DVD-RAM - no packet writing\n");
+			return 0;
+		case 0x13: /* DVD-RW restricted overwrite */
+			printk("pktcdvd: inserted media is DVD-RW with restricted overwrite");
 			return 1;
+		case 0x14: /* DVD-RW sequential recording */
+			printk("pktcdvd: inserted media is DVD-RW with sequential recording"
+			       " - no packet writing\n");
+			return 0;
+		case 0x1a: /* DVD+RW */
+			printk("pktcdvd: inserted media is DVD+RW\n");
+			return 1;
+		case 0xffff: /* unconforming */
+			printk("pktcdvd: inserted media does not conform to a known standard\n");
+			break;
+		default:
+			printk("pktcdvd: inserted media is yet UNKNOWN by pktcdvd\n");
+			return 0;
 	}
 
 	/*
@@ -1581,25 +1606,25 @@ static int pkt_good_disc(struct pktcdvd_
 	 */
 	if (di->disc_type == 0xff) {
 		printk("pktcdvd: Unknown disc. No track?\n");
-		return 1;
+		return 0;
 	}
 
 	if (di->disc_type != 0x20 && di->disc_type != 0) {
 		printk("pktcdvd: Wrong disc type (%x)\n", di->disc_type);
-		return 1;
+		return 0;
 	}
 
 	if (di->erasable == 0) {
 		printk("pktcdvd: Disc not erasable\n");
-		return 1;
+		return 0;
 	}
 
 	if (di->border_status == PACKET_SESSION_RESERVED) {
 		printk("pktcdvd: Can't write to last track (reserved)\n");
-		return 1;
+		return 0;
 	}
 
-	return 0;
+	return 1;
 }
 
 static int pkt_probe_settings(struct pktcdvd_device *pd)
@@ -1624,20 +1649,9 @@ static int pkt_probe_settings(struct pkt
 		return ret;
 	}
 
-	if (pkt_good_disc(pd, &di))
+	if (!pkt_good_disc(pd, &di))
 		return -ENXIO;
 
-	switch (pd->mmc3_profile) {
-		case 0x1a: /* DVD+RW */
-			printk("pktcdvd: inserted media is DVD+RW\n");
-			break;
-		case 0x13: /* DVD-RW */
-			printk("pktcdvd: inserted media is DVD-RW\n");
-			break;
-		default:
-			printk("pktcdvd: inserted media is CD-R%s\n", di.erasable ? "W" : "");
-			break;
-	}
 	pd->type = di.erasable ? PACKET_CDRW : PACKET_CDR;
 
 	track = 1; /* (di.last_track_msb << 8) | di.last_track_lsb; */


Regards,
Julien

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

* Re: Packet writing problems
  2004-08-18  8:38               ` Julien Oster
@ 2004-08-18  8:48                 ` Julien Oster
  0 siblings, 0 replies; 14+ messages in thread
From: Julien Oster @ 2004-08-18  8:48 UTC (permalink / raw)
  To: Peter Osterlund
  Cc: Julien Oster, Frediano Ziglio, axboe, linux-kernel, Andrew Morton

Julien Oster <usenet-20040502@usenet.frodoid.org> writes:

> So here's another patch instead which does what I meant.
>
> Additionally, it inverts the return values of pkt_good_disc, so that
> the condition later becomes "if (!pkt_good_disc(...))" (because
> actually, it is NOT a good disc then)

I forgot to change the comment above the function definition which
describes the return values. Here's a better patch. Additionally, it
also reverts the return values of pkt_good_track() for
convenience. (Same thing as with pkt_good_disc(), since the function
is named that way it should return 'false', i.e. 0, when the disc is
not a good disc for packet writing)

A quick question that came up while writing the patch: can't you
packet write DVD-RAM? I never had the opportunity to handle DVD-RAMs,
but the name suggests a random access memory...

--- fuzzy-2.6.8.1-orig/drivers/block/pktcdvd.c	2004-08-18 01:22:53.000000000 +0200
+++ fuzzy-2.6.8.1/drivers/block/pktcdvd.c	2004-08-18 10:44:26.445886873 +0200
@@ -1528,7 +1528,7 @@ static int pkt_set_write_settings(struct
 }
 
 /*
- * 0 -- we can write to this track, 1 -- we can't
+ * 1 -- we can write to this track, 0 -- we can't
  */
 static int pkt_good_track(track_information *ti)
 {
@@ -1540,39 +1540,64 @@ static int pkt_good_track(track_informat
 	 * FIXME: only for FP
 	 */
 	if (ti->fp == 0)
-		return 0;
+		return 1;
 
 	/*
 	 * "good" settings as per Mt Fuji.
 	 */
 	if (ti->rt == 0 && ti->blank == 0 && ti->packet == 1)
-		return 0;
+		return 1;
 
 	if (ti->rt == 0 && ti->blank == 1 && ti->packet == 1)
-		return 0;
+		return 1;
 
 	if (ti->rt == 1 && ti->blank == 0 && ti->packet == 1)
-		return 0;
+		return 1;
 
 	printk("pktcdvd: bad state %d-%d-%d\n", ti->rt, ti->blank, ti->packet);
-	return 1;
+	return 0;
 }
 
 /*
- * 0 -- we can write to this disc, 1 -- we can't
+ * 1 -- we can write to this disc, 0 -- we can't
  */
 static int pkt_good_disc(struct pktcdvd_device *pd, disc_information *di)
 {
 	switch (pd->mmc3_profile) {
-		case 0x0a: /* CD-RW */
-		case 0xffff: /* MMC3 not supported */
-			break;
-		case 0x1a: /* DVD+RW */
-		case 0x13: /* DVD-RW */
+	        case 0x08: /* CD-ROM */
+	                printk("pktcdvd: inserted media is CD-ROM - no packet writing\n");
 			return 0;
-		default:
-			printk("pktcdvd: Wrong disc profile (%x)\n", pd->mmc3_profile);
+	        case 0x09: /* CD-R */
+	                printk("pktcdvd: inserted media is CD-R - no packet writing\n");
+			return 0;
+	        case 0x0a: /* CD-RW */
+	                printk("pktcdvd: inserted media is CD-RW\n");
+			return 1;
+	        case 0x10: /* DVD-ROM */
+	                printk("pktcdvd: inserted media is DVD-ROM - no packet writing\n");
+			return 0;
+	        case 0x11: /* DVD-R */
+	                printk("pktcdvd: inserted media is DVD-R - no packet writing\n");
+			return 0;
+	        case 0x12: /* DVD-RAM */
+	                printk("pktcdvd: inserted media is DVD-RAM - no packet writing\n");
+			return 0;
+		case 0x13: /* DVD-RW restricted overwrite */
+			printk("pktcdvd: inserted media is DVD-RW with restricted overwrite");
+			return 1;
+		case 0x14: /* DVD-RW sequential recording */
+			printk("pktcdvd: inserted media is DVD-RW with sequential recording"
+			       " - no packet writing\n");
+			return 0;
+		case 0x1a: /* DVD+RW */
+			printk("pktcdvd: inserted media is DVD+RW\n");
 			return 1;
+		case 0xffff: /* unconforming */
+			printk("pktcdvd: inserted media does not conform to a known standard\n");
+			break;
+		default:
+			printk("pktcdvd: inserted media is yet UNKNOWN by pktcdvd\n");
+			return 0;
 	}
 
 	/*
@@ -1581,25 +1606,25 @@ static int pkt_good_disc(struct pktcdvd_
 	 */
 	if (di->disc_type == 0xff) {
 		printk("pktcdvd: Unknown disc. No track?\n");
-		return 1;
+		return 0;
 	}
 
 	if (di->disc_type != 0x20 && di->disc_type != 0) {
 		printk("pktcdvd: Wrong disc type (%x)\n", di->disc_type);
-		return 1;
+		return 0;
 	}
 
 	if (di->erasable == 0) {
 		printk("pktcdvd: Disc not erasable\n");
-		return 1;
+		return 0;
 	}
 
 	if (di->border_status == PACKET_SESSION_RESERVED) {
 		printk("pktcdvd: Can't write to last track (reserved)\n");
-		return 1;
+		return 0;
 	}
 
-	return 0;
+	return 1;
 }
 
 static int pkt_probe_settings(struct pktcdvd_device *pd)
@@ -1624,20 +1649,9 @@ static int pkt_probe_settings(struct pkt
 		return ret;
 	}
 
-	if (pkt_good_disc(pd, &di))
+	if (!pkt_good_disc(pd, &di))
 		return -ENXIO;
 
-	switch (pd->mmc3_profile) {
-		case 0x1a: /* DVD+RW */
-			printk("pktcdvd: inserted media is DVD+RW\n");
-			break;
-		case 0x13: /* DVD-RW */
-			printk("pktcdvd: inserted media is DVD-RW\n");
-			break;
-		default:
-			printk("pktcdvd: inserted media is CD-R%s\n", di.erasable ? "W" : "");
-			break;
-	}
 	pd->type = di.erasable ? PACKET_CDRW : PACKET_CDR;
 
 	track = 1; /* (di.last_track_msb << 8) | di.last_track_lsb; */
@@ -1646,7 +1660,7 @@ static int pkt_probe_settings(struct pkt
 		return ret;
 	}
 
-	if (pkt_good_track(&ti)) {
+	if (!pkt_good_track(&ti)) {
 		printk("pktcdvd: can't write to this track\n");
 		return -ENXIO;
 	}

Regards,
Julien

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

* Re: Packet writing problems
       [not found] <20040818125719.GA6021@linux-ari.internal>
@ 2004-08-18 17:08 ` Julien Oster
  2004-08-19  6:56   ` Alex Riesen
  0 siblings, 1 reply; 14+ messages in thread
From: Julien Oster @ 2004-08-18 17:08 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Linux Kernel Mailing List

Alex Riesen <ari@mbs-software.de> writes:

(I guess your mail was also meant to lkml?)

> this part has '\n' missing.

> BTW, all the printks have "pktcdvd: inserted media " prefixed and no
> arguments. Probably something like this can be useful:

> static void inserted_media(const char* desc)

[...]

> The purpose is clear without an extra comment.

Aaaand another one.

Maybe the following approach is nice?

We're making a lot of mess about some simple output.


--- fuzzy-2.6.8.1-orig/drivers/block/pktcdvd.c	2004-08-18 01:22:53.000000000 +0200
+++ fuzzy-2.6.8.1/drivers/block/pktcdvd.c	2004-08-18 18:59:41.136535859 +0200
@@ -1528,7 +1528,7 @@ static int pkt_set_write_settings(struct
 }
 
 /*
- * 0 -- we can write to this track, 1 -- we can't
+ * 1 -- we can write to this track, 0 -- we can't
  */
 static int pkt_good_track(track_information *ti)
 {
@@ -1540,66 +1540,111 @@ static int pkt_good_track(track_informat
 	 * FIXME: only for FP
 	 */
 	if (ti->fp == 0)
-		return 0;
+		return 1;
 
 	/*
 	 * "good" settings as per Mt Fuji.
 	 */
 	if (ti->rt == 0 && ti->blank == 0 && ti->packet == 1)
-		return 0;
+		return 1;
 
 	if (ti->rt == 0 && ti->blank == 1 && ti->packet == 1)
-		return 0;
+		return 1;
 
 	if (ti->rt == 1 && ti->blank == 0 && ti->packet == 1)
-		return 0;
+		return 1;
 
 	printk("pktcdvd: bad state %d-%d-%d\n", ti->rt, ti->blank, ti->packet);
-	return 1;
+	return 0;
 }
 
 /*
- * 0 -- we can write to this disc, 1 -- we can't
+ * 1 -- we can write to this disc, 0 -- we can't
  */
 static int pkt_good_disc(struct pktcdvd_device *pd, disc_information *di)
 {
+        char *mediatypename;
+	int packet_ok;
+
 	switch (pd->mmc3_profile) {
-		case 0x0a: /* CD-RW */
-		case 0xffff: /* MMC3 not supported */
+	        case 0x08: /* CD-ROM */
+		        mediatypename = "CD-ROM";
+			packet_ok = 0;
+			break;
+	        case 0x09: /* CD-R */
+		        mediatypename = "CD-R";
+			packet_ok = 0;
+			break;
+	        case 0x0a: /* CD-RW */
+		        mediatypename = "CD-RW";
+			packet_ok = 1;
+			break;
+	        case 0x10: /* DVD-ROM */
+		        mediatypename = "DVD-ROM";
+			packet_ok = 0;
+			break;
+	        case 0x11: /* DVD-R */
+		        mediatypename = "DVD-R";
+			packet_ok = 0;
+			break;
+	        case 0x12: /* DVD-RAM */
+		        mediatypename = "DVD-RAM";
+			packet_ok = 0;
+			break;
+		case 0x13: /* DVD-RW restricted overwrite */
+		        mediatypename = "DVD-RW w/ restricted overwrite";
+			packet_ok = 1;
+			break;
+		case 0x14: /* DVD-RW sequential recording */
+		        mediatypename = "DVD-RW w/ sequential recording";
+			packet_ok = 0;
 			break;
 		case 0x1a: /* DVD+RW */
-		case 0x13: /* DVD-RW */
-			return 0;
+		        mediatypename = "DVD+RW";
+			packet_ok = 1;
+			break;
+		case 0xffff: /* unconforming */
+		        mediatypename = "MMC unconforming";
+
+			/* handle it specially below */
+			packet_ok = 2;
+			break;
 		default:
-			printk("pktcdvd: Wrong disc profile (%x)\n", pd->mmc3_profile);
-			return 1;
+		        mediatypename = "unknown to pktcdvd";
+			packet_ok = 0;
 	}
 
+	printk("pktcdvd: inserted media is %s %s\n", mediatypename,
+	       packet_ok ? "" : "- no packet writing supported");
+
+	if (packet_ok != 2)
+	  return packet_ok;
+
 	/*
 	 * for disc type 0xff we should probably reserve a new track.
 	 * but i'm not sure, should we leave this to user apps? probably.
 	 */
 	if (di->disc_type == 0xff) {
 		printk("pktcdvd: Unknown disc. No track?\n");
-		return 1;
+		return 0;
 	}
 
 	if (di->disc_type != 0x20 && di->disc_type != 0) {
 		printk("pktcdvd: Wrong disc type (%x)\n", di->disc_type);
-		return 1;
+		return 0;
 	}
 
 	if (di->erasable == 0) {
 		printk("pktcdvd: Disc not erasable\n");
-		return 1;
+		return 0;
 	}
 
 	if (di->border_status == PACKET_SESSION_RESERVED) {
 		printk("pktcdvd: Can't write to last track (reserved)\n");
-		return 1;
+		return 0;
 	}
 
-	return 0;
+	return 1;
 }
 
 static int pkt_probe_settings(struct pktcdvd_device *pd)
@@ -1624,20 +1669,9 @@ static int pkt_probe_settings(struct pkt
 		return ret;
 	}
 
-	if (pkt_good_disc(pd, &di))
+	if (!pkt_good_disc(pd, &di))
 		return -ENXIO;
 
-	switch (pd->mmc3_profile) {
-		case 0x1a: /* DVD+RW */
-			printk("pktcdvd: inserted media is DVD+RW\n");
-			break;
-		case 0x13: /* DVD-RW */
-			printk("pktcdvd: inserted media is DVD-RW\n");
-			break;
-		default:
-			printk("pktcdvd: inserted media is CD-R%s\n", di.erasable ? "W" : "");
-			break;
-	}
 	pd->type = di.erasable ? PACKET_CDRW : PACKET_CDR;
 
 	track = 1; /* (di.last_track_msb << 8) | di.last_track_lsb; */
@@ -1646,7 +1680,7 @@ static int pkt_probe_settings(struct pkt
 		return ret;
 	}
 
-	if (pkt_good_track(&ti)) {
+	if (!pkt_good_track(&ti)) {
 		printk("pktcdvd: can't write to this track\n");
 		return -ENXIO;
 	}

Regards,
Julien

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

* Re: Packet writing problems
  2004-08-18 17:08 ` Julien Oster
@ 2004-08-19  6:56   ` Alex Riesen
  0 siblings, 0 replies; 14+ messages in thread
From: Alex Riesen @ 2004-08-19  6:56 UTC (permalink / raw)
  To: Linux Kernel Mailing List

Julien Oster, Wed, Aug 18, 2004 19:08:15 +0200:
> (I guess your mail was also meant to lkml?)

yes, I'm just reading the list off the archive at marc, so replies do
not go automatically to lkml if you have to copy-paste the text.

> We're making a lot of mess about some simple output.

Well, it's a lot of simple output.

>  static int pkt_good_disc(struct pktcdvd_device *pd, disc_information *di)
>  {
> +        char *mediatypename;

it is "const char*", actually. And the indentation before the line is
8 spaces, instead of 1 tab (unlike everything in the file).


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

end of thread, other threads:[~2004-08-19  6:57 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-08-16 15:16 Packet writing problems Frediano Ziglio
2004-08-16 18:50 ` Peter Osterlund
2004-08-16 19:09   ` Peter Osterlund
2004-08-16 19:51     ` Frediano Ziglio
2004-08-16 19:55     ` Frediano Ziglio
2004-08-17 19:59       ` Peter Osterlund
2004-08-17 20:24         ` Peter Osterlund
2004-08-17 23:31         ` Julien Oster
2004-08-17 23:36           ` Peter Osterlund
2004-08-18  8:25             ` Julien Oster
2004-08-18  8:38               ` Julien Oster
2004-08-18  8:48                 ` Julien Oster
     [not found] <20040818125719.GA6021@linux-ari.internal>
2004-08-18 17:08 ` Julien Oster
2004-08-19  6:56   ` Alex Riesen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox