All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] pktcdvd: Correctly set rq->cmd_len in pkt_generic_packet()
@ 2006-02-19 15:50 Peter Osterlund
  2006-02-19 15:53 ` [PATCH 2/5] pktcdvd: Rename functions and make their return values sane Peter Osterlund
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Osterlund @ 2006-02-19 15:50 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

It looks like the code in pkt_generic_packet() worked by luck in the
past, but after commit 186d330e682210100c671355580a8592e4a21692
leaving rq->cmd_len uninitialized doesn't work any more.

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

 drivers/block/pktcdvd.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index f783af7..5276d66 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -59,6 +59,7 @@
 #include <linux/mutex.h>
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_ioctl.h>
+#include <scsi/scsi.h>
 
 #include <asm/uaccess.h>
 
@@ -381,6 +382,7 @@ static int pkt_generic_packet(struct pkt
 	memcpy(rq->cmd, cgc->cmd, CDROM_PACKET_SIZE);
 	if (sizeof(rq->cmd) > CDROM_PACKET_SIZE)
 		memset(rq->cmd + CDROM_PACKET_SIZE, 0, sizeof(rq->cmd) - CDROM_PACKET_SIZE);
+	rq->cmd_len = COMMAND_SIZE(rq->cmd[0]);
 
 	rq->ref_count++;
 	rq->flags |= REQ_NOMERGE;

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

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

* [PATCH 2/5] pktcdvd: Rename functions and make their return values sane
  2006-02-19 15:50 [PATCH 1/5] pktcdvd: Correctly set rq->cmd_len in pkt_generic_packet() Peter Osterlund
@ 2006-02-19 15:53 ` Peter Osterlund
  2006-02-19 15:56   ` [PATCH 3/5] pktcdvd: Remove useless printk statements Peter Osterlund
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Osterlund @ 2006-02-19 15:53 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

Boolean functions should return non-zero when they mean "true",
otherwise the calling code looks weird. (As suggested by Linus.)

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

 drivers/block/pktcdvd.c |   36 ++++++++++++++++++------------------
 1 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 5276d66..12ff6d8 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -1498,9 +1498,9 @@ 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)
+static int pkt_writable_track(track_information *ti)
 {
 	/*
 	 * only good for CD-RW at the moment, not DVD-RW
@@ -1510,28 +1510,28 @@ 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)
+static int pkt_writable_disc(struct pktcdvd_device *pd, disc_information *di)
 {
 	switch (pd->mmc3_profile) {
 		case 0x0a: /* CD-RW */
@@ -1540,10 +1540,10 @@ static int pkt_good_disc(struct pktcdvd_
 		case 0x1a: /* DVD+RW */
 		case 0x13: /* DVD-RW */
 		case 0x12: /* DVD-RAM */
-			return 0;
+			return 1;
 		default:
 			VPRINTK("pktcdvd: Wrong disc profile (%x)\n", pd->mmc3_profile);
-			return 1;
+			return 0;
 	}
 
 	/*
@@ -1552,25 +1552,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)
@@ -1595,7 +1595,7 @@ static int pkt_probe_settings(struct pkt
 		return ret;
 	}
 
-	if (pkt_good_disc(pd, &di))
+	if (!pkt_writable_disc(pd, &di))
 		return -ENXIO;
 
 	switch (pd->mmc3_profile) {
@@ -1620,7 +1620,7 @@ static int pkt_probe_settings(struct pkt
 		return ret;
 	}
 
-	if (pkt_good_track(&ti)) {
+	if (!pkt_writable_track(&ti)) {
 		printk("pktcdvd: can't write to this track\n");
 		return -ENXIO;
 	}

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

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

* [PATCH 3/5] pktcdvd: Remove useless printk statements
  2006-02-19 15:53 ` [PATCH 2/5] pktcdvd: Rename functions and make their return values sane Peter Osterlund
@ 2006-02-19 15:56   ` Peter Osterlund
  2006-02-19 15:58     ` [PATCH 4/5] pktcdvd: Fix the logic in the pkt_writable_track function Peter Osterlund
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Osterlund @ 2006-02-19 15:56 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

Writing the detected disc type in the kernel log is not useful during
normal use of the driver, so remove the printk statements.

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

 drivers/block/pktcdvd.c |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 12ff6d8..dba5ce7 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -1598,20 +1598,6 @@ static int pkt_probe_settings(struct pkt
 	if (!pkt_writable_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;
-		case 0x12: /* DVD-RAM */
-			printk("pktcdvd: inserted media is DVD-RAM\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; */

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

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

* [PATCH 4/5] pktcdvd: Fix the logic in the pkt_writable_track function
  2006-02-19 15:56   ` [PATCH 3/5] pktcdvd: Remove useless printk statements Peter Osterlund
@ 2006-02-19 15:58     ` Peter Osterlund
  2006-02-19 16:00       ` [PATCH 5/5] pktcdvd: Only return -EROFS when appropriate Peter Osterlund
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Osterlund @ 2006-02-19 15:58 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

Fix the pkt_writable_track() function to make it work correctly for
all types of CD/DVD discs.

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

 drivers/block/pktcdvd.c |   28 +++++++++++++++-------------
 1 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index dba5ce7..dec68d0 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -1500,28 +1500,30 @@ static int pkt_set_write_settings(struct
 /*
  * 1 -- we can write to this track, 0 -- we can't
  */
-static int pkt_writable_track(track_information *ti)
+static int pkt_writable_track(struct pktcdvd_device *pd, track_information *ti)
 {
-	/*
-	 * only good for CD-RW at the moment, not DVD-RW
-	 */
+	switch (pd->mmc3_profile) {
+		case 0x1a: /* DVD+RW */
+		case 0x12: /* DVD-RAM */
+			/* The track is always writable on DVD+RW/DVD-RAM */
+			return 1;
+		default:
+			break;
+	}
 
-	/*
-	 * FIXME: only for FP
-	 */
-	if (ti->fp == 0)
-		return 1;
+	if (!ti->packet || !ti->fp)
+		return 0;
 
 	/*
 	 * "good" settings as per Mt Fuji.
 	 */
-	if (ti->rt == 0 && ti->blank == 0 && ti->packet == 1)
+	if (ti->rt == 0 && ti->blank == 0)
 		return 1;
 
-	if (ti->rt == 0 && ti->blank == 1 && ti->packet == 1)
+	if (ti->rt == 0 && ti->blank == 1)
 		return 1;
 
-	if (ti->rt == 1 && ti->blank == 0 && ti->packet == 1)
+	if (ti->rt == 1 && ti->blank == 0)
 		return 1;
 
 	printk("pktcdvd: bad state %d-%d-%d\n", ti->rt, ti->blank, ti->packet);
@@ -1606,7 +1608,7 @@ static int pkt_probe_settings(struct pkt
 		return ret;
 	}
 
-	if (!pkt_writable_track(&ti)) {
+	if (!pkt_writable_track(pd, &ti)) {
 		printk("pktcdvd: can't write to this track\n");
 		return -ENXIO;
 	}

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

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

* [PATCH 5/5] pktcdvd: Only return -EROFS when appropriate
  2006-02-19 15:58     ` [PATCH 4/5] pktcdvd: Fix the logic in the pkt_writable_track function Peter Osterlund
@ 2006-02-19 16:00       ` Peter Osterlund
  0 siblings, 0 replies; 5+ messages in thread
From: Peter Osterlund @ 2006-02-19 16:00 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

When attempting to open the device for writing, only return -EROFS if
the disc appears to be readable but not writable.

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

 drivers/block/pktcdvd.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index dec68d0..772b63c 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -1598,7 +1598,7 @@ static int pkt_probe_settings(struct pkt
 	}
 
 	if (!pkt_writable_disc(pd, &di))
-		return -ENXIO;
+		return -EROFS;
 
 	pd->type = di.erasable ? PACKET_CDRW : PACKET_CDR;
 
@@ -1610,7 +1610,7 @@ static int pkt_probe_settings(struct pkt
 
 	if (!pkt_writable_track(pd, &ti)) {
 		printk("pktcdvd: can't write to this track\n");
-		return -ENXIO;
+		return -EROFS;
 	}
 
 	/*
@@ -1624,7 +1624,7 @@ static int pkt_probe_settings(struct pkt
 	}
 	if (pd->settings.size > PACKET_MAX_SECTORS) {
 		printk("pktcdvd: packet size is too big\n");
-		return -ENXIO;
+		return -EROFS;
 	}
 	pd->settings.fp = ti.fp;
 	pd->offset = (be32_to_cpu(ti.track_start) << 2) & (pd->settings.size - 1);
@@ -1666,7 +1666,7 @@ static int pkt_probe_settings(struct pkt
 			break;
 		default:
 			printk("pktcdvd: unknown data mode\n");
-			return 1;
+			return -EROFS;
 	}
 	return 0;
 }
@@ -1877,7 +1877,7 @@ static int pkt_open_write(struct pktcdvd
 
 	if ((ret = pkt_probe_settings(pd))) {
 		VPRINTK("pktcdvd: %s failed probe\n", pd->name);
-		return -EROFS;
+		return ret;
 	}
 
 	if ((ret = pkt_set_write_settings(pd))) {

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

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

end of thread, other threads:[~2006-02-19 16:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-02-19 15:50 [PATCH 1/5] pktcdvd: Correctly set rq->cmd_len in pkt_generic_packet() Peter Osterlund
2006-02-19 15:53 ` [PATCH 2/5] pktcdvd: Rename functions and make their return values sane Peter Osterlund
2006-02-19 15:56   ` [PATCH 3/5] pktcdvd: Remove useless printk statements Peter Osterlund
2006-02-19 15:58     ` [PATCH 4/5] pktcdvd: Fix the logic in the pkt_writable_track function Peter Osterlund
2006-02-19 16:00       ` [PATCH 5/5] pktcdvd: Only return -EROFS when appropriate Peter Osterlund

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.