* [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.