* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox