LKML Archive on lore.kernel.org help / color / mirror / Atom feed
* [PATCH 0/5] ide-cd: trivial fixes
@ 2008-02-27 18:20 Borislav Petkov
2008-02-27 18:20 ` [PATCH 1/5] ide-cd: include proper headers Borislav Petkov
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Borislav Petkov @ 2008-02-27 18:20 UTC (permalink / raw)
To: bzolnier; +Cc: linux-kernel, linux-ide, Borislav Petkov
Hi Bart,
here are the ide-cd trivial fixes split in an easier to digest format. I
cannot do the md5sum check here since all of the patches change at least one
char but i hope that this'll be redundant here since they're trivially easy
to review :) now.
drivers/ide/ide-cd.c | 591 +++++++++++++++++++++++++-------------------------
1 files changed, 298 insertions(+), 293 deletions(-)
Thanks,
Boris.
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/5] ide-cd: include proper headers 2008-02-27 18:20 [PATCH 0/5] ide-cd: trivial fixes Borislav Petkov @ 2008-02-27 18:20 ` Borislav Petkov 2008-02-27 18:20 ` [PATCH 2/5] ide-cd: put all proc-related code at one place Borislav Petkov ` (4 subsequent siblings) 5 siblings, 0 replies; 17+ messages in thread From: Borislav Petkov @ 2008-02-27 18:20 UTC (permalink / raw) To: bzolnier; +Cc: linux-kernel, linux-ide, Borislav Petkov Signed-off-by: Borislav Petkov <petkovbb@gmail.com> --- drivers/ide/ide-cd.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index 8877b7b..4d39567 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -41,10 +41,10 @@ #include <scsi/scsi.h> /* For SCSI -> ATAPI command conversion */ -#include <asm/irq.h> -#include <asm/io.h> +#include <linux/irq.h> +#include <linux/io.h> #include <asm/byteorder.h> -#include <asm/uaccess.h> +#include <linux/uaccess.h> #include <asm/unaligned.h> #include "ide-cd.h" -- 1.5.4.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/5] ide-cd: put all proc-related code at one place 2008-02-27 18:20 [PATCH 0/5] ide-cd: trivial fixes Borislav Petkov 2008-02-27 18:20 ` [PATCH 1/5] ide-cd: include proper headers Borislav Petkov @ 2008-02-27 18:20 ` Borislav Petkov 2008-02-27 18:20 ` [PATCH 3/5] ide-cd: fixup comments Borislav Petkov ` (3 subsequent siblings) 5 siblings, 0 replies; 17+ messages in thread From: Borislav Petkov @ 2008-02-27 18:20 UTC (permalink / raw) To: bzolnier; +Cc: linux-kernel, linux-ide, Borislav Petkov Signed-off-by: Borislav Petkov <petkovbb@gmail.com> --- drivers/ide/ide-cd.c | 71 ++++++++++++++++++++++++------------------------- 1 files changed, 35 insertions(+), 36 deletions(-) diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index 4d39567..bb7944b 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -1695,15 +1695,6 @@ int ide_cdrom_probe_capabilities(ide_drive_t *drive) return nslots; } -#ifdef CONFIG_IDE_PROC_FS -static void ide_cdrom_add_settings(ide_drive_t *drive) -{ - ide_add_setting(drive, "dsc_overlap", SETTING_RW, TYPE_BYTE, 0, 1, 1, 1, &drive->dsc_overlap, NULL); -} -#else -static inline void ide_cdrom_add_settings(ide_drive_t *drive) { ; } -#endif - /* * standard prep_rq_fn that builds 10 byte cmds */ @@ -1789,6 +1780,41 @@ struct cd_list_entry { unsigned int cd_flags; }; +#ifdef CONFIG_IDE_PROC_FS +static sector_t ide_cdrom_capacity(ide_drive_t *drive) +{ + unsigned long capacity, sectors_per_frame; + + if (cdrom_read_capacity(drive, &capacity, §ors_per_frame, NULL)) + return 0; + + return capacity * sectors_per_frame; +} + +static int proc_idecd_read_capacity + (char *page, char **start, off_t off, int count, int *eof, void *data) +{ + ide_drive_t *drive = data; + int len; + + len = sprintf(page, "%llu\n", (long long)ide_cdrom_capacity(drive)); + PROC_IDE_READ_RETURN(page, start, off, count, eof, len); +} + +static ide_proc_entry_t idecd_proc[] = { + { "capacity", S_IFREG|S_IRUGO, proc_idecd_read_capacity, NULL }, + { NULL, 0, NULL, NULL } +}; + +static void ide_cdrom_add_settings(ide_drive_t *drive) +{ + ide_add_setting(drive, "dsc_overlap", SETTING_RW, TYPE_BYTE, 0, 1, 1, 1, + &drive->dsc_overlap, NULL); +} +#else +static inline void ide_cdrom_add_settings(ide_drive_t *drive) { ; } +#endif + static const struct cd_list_entry ide_cd_quirks_list[] = { /* Limit transfer size per interrupt. */ { "SAMSUNG CD-ROM SCR-2430", NULL, IDE_CD_FLAG_LIMIT_NFRAMES }, @@ -1930,33 +1956,6 @@ static void ide_cd_release(struct kref *kref) static int ide_cd_probe(ide_drive_t *); -#ifdef CONFIG_IDE_PROC_FS -static sector_t ide_cdrom_capacity(ide_drive_t *drive) -{ - unsigned long capacity, sectors_per_frame; - - if (cdrom_read_capacity(drive, &capacity, §ors_per_frame, NULL)) - return 0; - - return capacity * sectors_per_frame; -} - -static int proc_idecd_read_capacity - (char *page, char **start, off_t off, int count, int *eof, void *data) -{ - ide_drive_t *drive = data; - int len; - - len = sprintf(page, "%llu\n", (long long)ide_cdrom_capacity(drive)); - PROC_IDE_READ_RETURN(page, start, off, count, eof, len); -} - -static ide_proc_entry_t idecd_proc[] = { - { "capacity", S_IFREG|S_IRUGO, proc_idecd_read_capacity, NULL }, - { NULL, 0, NULL, NULL } -}; -#endif - static ide_driver_t ide_cdrom_driver = { .gen_driver = { .owner = THIS_MODULE, -- 1.5.4.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/5] ide-cd: fixup comments 2008-02-27 18:20 [PATCH 0/5] ide-cd: trivial fixes Borislav Petkov 2008-02-27 18:20 ` [PATCH 1/5] ide-cd: include proper headers Borislav Petkov 2008-02-27 18:20 ` [PATCH 2/5] ide-cd: put all proc-related code at one place Borislav Petkov @ 2008-02-27 18:20 ` Borislav Petkov 2008-02-27 21:18 ` Bartlomiej Zolnierkiewicz 2008-02-27 18:20 ` [PATCH 4/5] ide-cd: shorten lines longer than 80 columns Borislav Petkov ` (2 subsequent siblings) 5 siblings, 1 reply; 17+ messages in thread From: Borislav Petkov @ 2008-02-27 18:20 UTC (permalink / raw) To: bzolnier; +Cc: linux-kernel, linux-ide, Borislav Petkov [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain, Size: 35034 bytes --] Signed-off-by: Borislav Petkov <petkovbb@gmail.com> --- drivers/ide/ide-cd.c | 401 ++++++++++++++++++++++++-------------------------- 1 files changed, 192 insertions(+), 209 deletions(-) diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index bb7944b..bb501a4 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -39,7 +39,8 @@ #include <linux/mutex.h> #include <linux/bcd.h> -#include <scsi/scsi.h> /* For SCSI -> ATAPI command conversion */ +/* For SCSI -> ATAPI command conversion */ +#include <scsi/scsi.h> #include <linux/irq.h> #include <linux/io.h> @@ -77,12 +78,11 @@ static void ide_cd_put(struct cdrom_info *cd) mutex_unlock(&idecd_ref_mutex); } -/**************************************************************************** +/* * Generic packet command support and error handling routines. */ -/* Mark that we've seen a media change, and invalidate our internal - buffers. */ +/* Mark that we've seen a media change and invalidate our internal buffers. */ static void cdrom_saw_media_change(ide_drive_t *drive) { struct cdrom_info *cd = drive->driver_data; @@ -105,9 +105,8 @@ static int cdrom_log_sense(ide_drive_t *drive, struct request *rq, break; case NOT_READY: /* - * don't care about tray state messages for - * e.g. capacity commands or in-progress or - * becoming ready + * don't care about tray state messages for e.g. capacity + * commands or in-progress or becoming ready */ if (sense->asc == 0x3a || sense->asc == 0x04) break; @@ -115,8 +114,8 @@ static int cdrom_log_sense(ide_drive_t *drive, struct request *rq, break; case ILLEGAL_REQUEST: /* - * don't log START_STOP unit with LoEj set, since - * we cannot reliably check if drive can auto-close + * don't log START_STOP unit with LoEj set, since we cannot + * reliably check if drive can auto-close */ if (rq->cmd[0] == GPCMD_START_STOP_UNIT && sense->asc == 0x24) break; @@ -124,9 +123,9 @@ static int cdrom_log_sense(ide_drive_t *drive, struct request *rq, break; case UNIT_ATTENTION: /* - * Make good and sure we've seen this potential media - * change. Some drives (i.e. Creative) fail to present - * the correct sense key in the error register. + * Make good and sure we've seen this potential media change. + * Some drives (i.e. Creative) fail to present the correct sense + * key in the error register. */ cdrom_saw_media_change(drive); break; @@ -151,15 +150,16 @@ void cdrom_analyze_sense_data(ide_drive_t *drive, return; /* - * If a read toc is executed for a CD-R or CD-RW medium where - * the first toc has not been recorded yet, it will fail with - * 05/24/00 (which is a confusing error) + * If a read toc is executed for a CD-R or CD-RW medium where the first + * toc has not been recorded yet, it will fail with 05/24/00 (which is a + * confusing error) */ if (failed_command && failed_command->cmd[0] == GPCMD_READ_TOC_PMA_ATIP) if (sense->sense_key == 0x05 && sense->asc == 0x24) return; - if (sense->error_code == 0x70) { /* Current Error */ + /* current error */ + if (sense->error_code == 0x70) { switch (sense->sense_key) { case MEDIUM_ERROR: case VOLUME_OVERFLOW: @@ -178,7 +178,8 @@ void cdrom_analyze_sense_data(ide_drive_t *drive, if (bio_sectors < 4) bio_sectors = 4; if (drive->queue->hardsect_size == 2048) - sector <<= 2; /* Device sector size is 2K */ + /* device sector size is 2K */ + sector <<= 2; sector &= ~(bio_sectors - 1); valid = (sector - failed_command->sector) << 9; @@ -194,9 +195,7 @@ void cdrom_analyze_sense_data(ide_drive_t *drive, ide_cd_log_error(drive->name, failed_command, sense); } -/* - * Initialize a ide-cd packet command request - */ +/* Initialize a ide-cd packet command request */ void ide_cd_init_rq(ide_drive_t *drive, struct request *rq) { struct cdrom_info *cd = drive->driver_data; @@ -252,7 +251,7 @@ static void cdrom_end_request(ide_drive_t *drive, int uptodate) } cdrom_analyze_sense_data(drive, failed, sense); /* - * now end failed request + * now end the failed request */ if (blk_fs_request(failed)) { if (ide_end_dequeued_request(drive, failed, 0, @@ -287,14 +286,17 @@ static void ide_dump_status_no_sense(ide_drive_t *drive, const char *msg, u8 sta ide_dump_status(drive, msg, stat); } -/* Returns 0 if the request should be continued. - Returns 1 if the request was ended. */ +/* + * Returns: + * 0: if the request should be continued. + * 1: if the request was ended. + */ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) { struct request *rq = HWGROUP(drive)->rq; int stat, err, sense_key; - /* Check for errors. */ + /* check for errors. */ stat = ide_read_status(drive); if (stat_ret) @@ -303,7 +305,7 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) if (OK_STAT(stat, good_stat, BAD_R_STAT)) return 0; - /* Get the IDE error register. */ + /* get the IDE error register. */ err = ide_read_error(drive); sense_key = err >> 4; @@ -313,10 +315,11 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) } if (blk_sense_request(rq)) { - /* We got an error trying to get sense info - from the drive (probably while trying - to recover from a former error). Just give up. */ - + /* + * We got an error trying to get sense info from the drive + * (probably while trying to recover from a former error). + * Just give up. + */ rq->cmd_flags |= REQ_FAILED; cdrom_end_request(drive, 0); ide_error(drive, "request sense failure", stat); @@ -332,13 +335,12 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) if (blk_pc_request(rq) && !rq->errors) rq->errors = SAM_STAT_CHECK_CONDITION; - /* Check for tray open. */ + /* check for tray open. */ if (sense_key == NOT_READY) { cdrom_saw_media_change(drive); } else if (sense_key == UNIT_ATTENTION) { - /* Check for media change. */ + /* check for media change. */ cdrom_saw_media_change(drive); - /*printk("%s: media changed\n",drive->name);*/ return 0; } else if (sense_key == ILLEGAL_REQUEST && rq->cmd[0] == GPCMD_START_STOP_UNIT) { @@ -350,7 +352,7 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) * cdrom_log_sense() knows this! */ } else if (!(rq->cmd_flags & REQ_QUIET)) { - /* Otherwise, print an error. */ + /* otherwise, print an error. */ ide_dump_status(drive, "packet command error", stat); } @@ -366,25 +368,27 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) } else if (blk_fs_request(rq)) { int do_end_request = 0; - /* Handle errors from READ and WRITE requests. */ + /* handle errors from READ and WRITE requests. */ if (blk_noretry_request(rq)) do_end_request = 1; if (sense_key == NOT_READY) { - /* Tray open. */ + /* tray open. */ if (rq_data_dir(rq) == READ) { cdrom_saw_media_change(drive); - /* Fail the request. */ + /* fail the request. */ printk("%s: tray open\n", drive->name); do_end_request = 1; } else { struct cdrom_info *info = drive->driver_data; - /* allow the drive 5 seconds to recover, some + /* + * allow the drive 5 seconds to recover, some * devices will return this error while flushing - * data from cache */ + * data from cache. + */ if (!rq->errors) info->write_timeout = jiffies + ATAPI_WAIT_WRITE_BUSY; rq->errors = 1; @@ -394,8 +398,8 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) unsigned long flags; /* - * take a breather relying on the - * unplug timer to kick us again + * take a breather relying on the unplug + * timer to kick us again */ spin_lock_irqsave(&ide_lock, flags); blk_plug_device(drive->queue); @@ -404,55 +408,54 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) } } } else if (sense_key == UNIT_ATTENTION) { - /* Media change. */ + /* media change. */ cdrom_saw_media_change (drive); /* - * Arrange to retry the request. - * But be sure to give up if we've retried - * too many times. + * Arrange to retry the request but be sure to give up + * if we've retried too many times. */ if (++rq->errors > ERROR_MAX) do_end_request = 1; } else if (sense_key == ILLEGAL_REQUEST || sense_key == DATA_PROTECT) { /* - * No point in retrying after an illegal - * request or data protect error. + * No point in retrying after an illegal request or data + * protect error. */ ide_dump_status_no_sense(drive, "command error", stat); do_end_request = 1; } else if (sense_key == MEDIUM_ERROR) { /* * No point in re-trying a zillion times on a bad - * sector... If we got here the error is not correctable + * sector. If we got here the error is not correctable. */ ide_dump_status_no_sense(drive, "media error (bad sector)", stat); do_end_request = 1; } else if (sense_key == BLANK_CHECK) { - /* Disk appears blank ?? */ + /* disk appears blank ?? */ ide_dump_status_no_sense(drive, "media error (blank)", stat); do_end_request = 1; } else if ((err & ~ABRT_ERR) != 0) { - /* Go to the default handler - for other errors. */ + /* go to the default handler for other errors. */ ide_error(drive, "cdrom_decode_status", stat); return 1; } else if ((++rq->errors > ERROR_MAX)) { - /* We've racked up too many retries. Abort. */ + /* we've racked up too many retries, abort. */ do_end_request = 1; } - /* End a request through request sense analysis when we have - sense data. We need this in order to perform end of media - processing */ - + /* + * End a request through request sense analysis when we have + * sense data. We need this in order to perform end of media + * processing. + */ if (do_end_request) goto end_request; /* - * If we got a CHECK_CONDITION status, - * queue a request sense command. + * If we got a CHECK_CONDITION status, queue + * a request sense command. */ if (stat & ERR_STAT) cdrom_queue_request_sense(drive, NULL, NULL); @@ -461,7 +464,7 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) cdrom_end_request(drive, 0); } - /* Retry, or handle the next request. */ + /* retry, or handle the next request. */ return 1; end_request: @@ -486,10 +489,10 @@ static int cdrom_timer_expiry(ide_drive_t *drive) unsigned long wait = 0; /* - * Some commands are *slow* and normally take a long time to - * complete. Usually we can use the ATAPI "disconnect" to bypass - * this, but not all commands/drives support that. Let - * ide_timer_expiry keep polling us for these. + * Some commands are *slow* and normally take a long time to complete. + * Usually we can use the ATAPI "disconnect" to bypass this, but not all + * commands/drives support that. Let ide_timer_expiry keep polling us + * for these. */ switch (rq->cmd[0]) { case GPCMD_BLANK: @@ -508,13 +511,14 @@ static int cdrom_timer_expiry(ide_drive_t *drive) return wait; } -/* Set up the device registers for transferring a packet command on DEV, - expecting to later transfer XFERLEN bytes. HANDLER is the routine - which actually transfers the command to the drive. If this is a - drq_interrupt device, this routine will arrange for HANDLER to be - called when the interrupt from the drive arrives. Otherwise, HANDLER - will be called immediately after the drive is prepared for the transfer. */ - +/* + * Set up the device registers for transferring a packet command on DEV, + * expecting to later transfer XFERLEN bytes. HANDLER is the routine + * which actually transfers the command to the drive. If this is a + * drq_interrupt device, this routine will arrange for HANDLER to be + * called when the interrupt from the drive arrives. Otherwise, HANDLER + * will be called immediately after the drive is prepared for the transfer. + */ static ide_startstop_t cdrom_start_packet_command(ide_drive_t *drive, int xferlen, ide_handler_t *handler) @@ -523,7 +527,7 @@ static ide_startstop_t cdrom_start_packet_command(ide_drive_t *drive, struct cdrom_info *info = drive->driver_data; ide_hwif_t *hwif = drive->hwif; - /* Wait for the controller to be idle. */ + /* wait for the controller to be idle. */ if (ide_wait_stat(&startstop, drive, 0, BUSY_STAT, WAIT_READY)) return startstop; @@ -531,7 +535,7 @@ static ide_startstop_t cdrom_start_packet_command(ide_drive_t *drive, if (info->dma) info->dma = !hwif->dma_setup(drive); - /* Set up the controller registers. */ + /* set up the controller registers. */ ide_pktcmd_tf_load(drive, IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL | IDE_TFLAG_NO_SELECT_MASK, xferlen, info->dma); @@ -557,11 +561,12 @@ static ide_startstop_t cdrom_start_packet_command(ide_drive_t *drive, } } -/* Send a packet command to DRIVE described by CMD_BUF and CMD_LEN. - The device registers must have already been prepared - by cdrom_start_packet_command. - HANDLER is the interrupt handler to call when the command completes - or there's data ready. */ +/* + * Send a packet command to DRIVE described by CMD_BUF and CMD_LEN. The device + * registers must have already been prepared by cdrom_start_packet_command. + * HANDLER is the interrupt handler to call when the command completes or + * there's data ready. + */ #define ATAPI_MIN_CDB_BYTES 12 static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive, struct request *rq, @@ -573,24 +578,26 @@ static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive, ide_startstop_t startstop; if (info->cd_flags & IDE_CD_FLAG_DRQ_INTERRUPT) { - /* Here we should have been called after receiving an interrupt - from the device. DRQ should how be set. */ + /* + * Here we should have been called after receiving an interrupt + * from the device. DRQ should how be set. + */ - /* Check for errors. */ + /* check for errors. */ if (cdrom_decode_status(drive, DRQ_STAT, NULL)) return ide_stopped; - /* Ok, next interrupt will be DMA interrupt. */ + /* ok, next interrupt will be DMA interrupt. */ if (info->dma) drive->waiting_for_dma = 1; } else { - /* Otherwise, we must wait for DRQ to get set. */ + /* otherwise, we must wait for DRQ to get set. */ if (ide_wait_stat(&startstop, drive, DRQ_STAT, BUSY_STAT, WAIT_READY)) return startstop; } - /* Arm the interrupt handler. */ + /* arm the interrupt handler. */ ide_set_handler(drive, handler, rq->timeout, cdrom_timer_expiry); /* ATAPI commands get padded out to 12 bytes minimum */ @@ -598,20 +605,19 @@ static ide_startstop_t cdrom_transfer_packet_command(ide_drive_t *drive, if (cmd_len < ATAPI_MIN_CDB_BYTES) cmd_len = ATAPI_MIN_CDB_BYTES; - /* Send the command to the device. */ + /* send the command to the device. */ HWIF(drive)->atapi_output_bytes(drive, rq->cmd, cmd_len); - /* Start the DMA if need be */ + /* start the DMA if need be */ if (info->dma) hwif->dma_start(drive); return ide_started; } -/**************************************************************************** +/* * Block read functions. */ - static void ide_cd_pad_transfer(ide_drive_t *drive, xfer_func_t *xf, int len) { while (len > 0) { @@ -649,20 +655,21 @@ static int ide_cd_check_ireason(ide_drive_t *drive, struct request *rq, ide_hwif_t *hwif = drive->hwif; xfer_func_t *xf; - /* Whoops... */ + /* whoops... */ printk(KERN_ERR "%s: %s: wrong transfer direction!\n", drive->name, __func__); xf = rw ? hwif->atapi_output_bytes : hwif->atapi_input_bytes; ide_cd_pad_transfer(drive, xf, len); } else if (rw == 0 && ireason == 1) { - /* Some drives (ASUS) seem to tell us that status - * info is available. just get it and ignore. + /* + * Some drives (ASUS) seem to tell us that status info is + * available. just get it and ignore. */ (void)ide_read_status(drive); return 0; } else { - /* Drive wants a command packet, or invalid ireason... */ + /* drive wants a command packet, or invalid ireason... */ printk(KERN_ERR "%s: %s: bad interrupt reason 0x%02x\n", drive->name, __func__, ireason); } @@ -702,10 +709,10 @@ static int ide_cd_check_transfer_size(ide_drive_t *drive, int len) static ide_startstop_t cdrom_newpc_intr(ide_drive_t *); /* - * Routine to send a read/write packet command to the drive. - * This is usually called directly from cdrom_start_{read,write}(). - * However, for drq_interrupt devices, it is called from an interrupt - * when the drive is ready to accept the command. + * Routine to send a read/write packet command to the drive. This is usually + * called directly from cdrom_start_{read,write}(). However, for drq_interrupt + * devices, it is called from an interrupt when the drive is ready to accept + * the command. */ static ide_startstop_t cdrom_start_rw_cont(ide_drive_t *drive) { @@ -727,7 +734,7 @@ static ide_startstop_t cdrom_start_rw_cont(ide_drive_t *drive) * is larger than the buffer size. */ if (nskip > 0) { - /* Sanity check... */ + /* sanity check... */ if (rq->current_nr_sectors != bio_cur_sectors(rq->bio)) { printk(KERN_ERR "%s: %s: buffer botch (%u)\n", @@ -744,10 +751,10 @@ static ide_startstop_t cdrom_start_rw_cont(ide_drive_t *drive) /* the immediate bit */ rq->cmd[1] = 1 << 3; #endif - /* Set up the command */ + /* set up the command */ rq->timeout = ATAPI_WAIT_PC; - /* Send the command to the drive and return. */ + /* send the command to the drive and return. */ return cdrom_transfer_packet_command(drive, rq, cdrom_newpc_intr); } @@ -768,12 +775,7 @@ static ide_startstop_t cdrom_seek_intr(ide_drive_t *drive) if (retry && time_after(jiffies, info->start_seek + IDECD_SEEK_TIMER)) { if (--retry == 0) { - /* - * this condition is far too common, to bother - * users about it - */ - /* printk("%s: disabled DSC seek overlap\n", drive->name);*/ - drive->dsc_overlap = 0; + drive->dsc_overlap = 0; } } return ide_stopped; @@ -804,8 +806,8 @@ static ide_startstop_t cdrom_start_seek(ide_drive_t *drive, unsigned int block) } /* - * Fix up a possibly partially-processed request so that we can - * start it over entirely, or even put it back on the request queue. + * Fix up a possibly partially-processed request so that we can start it over + * entirely, or even put it back on the request queue. */ static void restore_request(struct request *rq) { @@ -822,10 +824,9 @@ static void restore_request(struct request *rq) rq->q->prep_rq_fn(rq->q, rq); } -/**************************************************************************** - * Execute all other packet commands. +/* + * All other packet commands. */ - static void ide_cd_request_sense_fixup(struct request *rq) { /* @@ -849,7 +850,7 @@ int ide_cd_queue_pc(ide_drive_t *drive, struct request *rq) if (rq->sense == NULL) rq->sense = &sense; - /* Start of retry loop. */ + /* start of retry loop. */ do { int error; unsigned long time = jiffies; @@ -858,41 +859,45 @@ int ide_cd_queue_pc(ide_drive_t *drive, struct request *rq) error = ide_do_drive_cmd(drive, rq, ide_wait); time = jiffies - time; - /* FIXME: we should probably abort/retry or something - * in case of failure */ + /* + * FIXME: we should probably abort/retry or something in case of + * failure. + */ if (rq->cmd_flags & REQ_FAILED) { - /* The request failed. Retry if it was due to a unit - attention status - (usually means media was changed). */ + /* + * The request failed. Retry if it was due to a unit + * attention status (usually means media was changed). + */ struct request_sense *reqbuf = rq->sense; if (reqbuf->sense_key == UNIT_ATTENTION) cdrom_saw_media_change(drive); else if (reqbuf->sense_key == NOT_READY && reqbuf->asc == 4 && reqbuf->ascq != 4) { - /* The drive is in the process of loading - a disk. Retry, but wait a little to give - the drive time to complete the load. */ + /* + * The drive is in the process of loading + * a disk. Retry, but wait a little to give + * the drive time to complete the load. + */ ssleep(2); } else { - /* Otherwise, don't retry. */ + /* otherwise, don't retry. */ retries = 0; } --retries; } - /* End of retry loop. */ + /* end of retry loop. */ } while ((rq->cmd_flags & REQ_FAILED) && retries >= 0); - /* Return an error if the command failed. */ + /* return an error if the command failed. */ return (rq->cmd_flags & REQ_FAILED) ? -EIO : 0; } /* - * Called from blk_end_request_callback() after the data of the request - * is completed and before the request is completed. - * By returning value '1', blk_end_request_callback() returns immediately - * without completing the request. + * Called from blk_end_request_callback() after the data of the request is + * completed and before the request itself is completed. By returning value '1', + * blk_end_request_callback() returns immediately without completing it. */ static int cdrom_newpc_intr_dummy_cb(struct request *rq) { @@ -911,7 +916,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive) unsigned int timeout; u8 lowcyl, highcyl; - /* Check for errors. */ + /* check for errors. */ dma = info->dma; if (dma) { info->dma = 0; @@ -926,9 +931,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive) if (cdrom_decode_status(drive, 0, &stat)) return ide_stopped; - /* - * using dma, transfer is complete now - */ + /* using dma, transfer is complete now */ if (dma) { if (dma_error) return ide_error(drive, "dma error", stat); @@ -939,9 +942,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive) goto end_request; } - /* - * ok we fall to pio :/ - */ + /* ok we fall to pio :/ */ ireason = hwif->INB(hwif->io_ports[IDE_IREASON_OFFSET]) & 0x3; lowcyl = hwif->INB(hwif->io_ports[IDE_BCOUNTL_OFFSET]); highcyl = hwif->INB(hwif->io_ports[IDE_BCOUNTH_OFFSET]); @@ -952,9 +953,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive) if (thislen > len) thislen = len; - /* - * If DRQ is clear, the command has completed. - */ + /* If DRQ is clear, the command has completed. */ if ((stat & DRQ_STAT) == 0) { if (blk_fs_request(rq)) { /* @@ -975,15 +974,13 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive) return ide_stopped; } else if (!blk_pc_request(rq)) { ide_cd_request_sense_fixup(rq); - /* Complain if we still have data left to transfer. */ + /* complain if we still have data left to transfer. */ uptodate = rq->data_len ? 0 : 1; } goto end_request; } - /* - * check which way to transfer data - */ + /* check which way to transfer data */ if (ide_cd_check_ireason(drive, rq, len, ireason, write)) return ide_stopped; @@ -1019,16 +1016,12 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive) xferfunc = HWIF(drive)->atapi_input_bytes; } - /* - * transfer data - */ + /* transfer data */ while (thislen > 0) { u8 *ptr = blk_fs_request(rq) ? NULL : rq->data; int blen = rq->data_len; - /* - * bio backed? - */ + /* bio backed? */ if (rq->bio) { if (blk_fs_request(rq)) { ptr = rq->buffer; @@ -1043,7 +1036,8 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive) if (blk_fs_request(rq) && !write) /* * If the buffers are full, pipe the rest into - * oblivion. */ + * oblivion. + */ ide_cd_drain_data(drive, thislen >> 9); else { printk(KERN_ERR "%s: confused, missing data\n", @@ -1090,9 +1084,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive) rq->sense_len += blen; } - /* - * pad, if necessary - */ + /* pad, if necessary */ if (!blk_fs_request(rq) && len > 0) ide_cd_pad_transfer(drive, xferfunc, len); @@ -1136,9 +1128,7 @@ static ide_startstop_t cdrom_start_rw(ide_drive_t *drive, struct request *rq) queue_hardsect_size(drive->queue) >> SECTOR_BITS; if (write) { - /* - * disk has become write protected - */ + /* disk has become write protected */ if (cd->disk->policy) { cdrom_end_request(drive, 0); return ide_stopped; @@ -1151,9 +1141,7 @@ static ide_startstop_t cdrom_start_rw(ide_drive_t *drive, struct request *rq) restore_request(rq); } - /* - * use DMA, if possible / writes *must* be hardware frame aligned - */ + /* use DMA, if possible / writes *must* be hardware frame aligned */ if ((rq->nr_sectors & (sectors_per_frame - 1)) || (rq->sector & (sectors_per_frame - 1))) { if (write) { @@ -1167,7 +1155,7 @@ static ide_startstop_t cdrom_start_rw(ide_drive_t *drive, struct request *rq) if (write) cd->devinfo.media_written = 1; - /* Start sending the read/write request to the drive. */ + /* start sending the read/write request to the drive. */ return cdrom_start_packet_command(drive, 32768, cdrom_start_rw_cont); } @@ -1192,9 +1180,7 @@ static ide_startstop_t cdrom_do_block_pc(ide_drive_t *drive, struct request *rq) info->dma = 0; - /* - * sg request - */ + /* sg request */ if (rq->bio) { int mask = drive->queue->dma_alignment; unsigned long addr = (unsigned long) page_address(bio_page(rq->bio)); @@ -1211,11 +1197,11 @@ static ide_startstop_t cdrom_do_block_pc(ide_drive_t *drive, struct request *rq) info->dma = 0; } - /* Start sending the command to the drive. */ + /* start sending the command to the drive. */ return cdrom_start_packet_command(drive, rq->data_len, cdrom_do_newpc_cont); } -/**************************************************************************** +/* * cdrom driver request routine. */ static ide_startstop_t @@ -1248,9 +1234,8 @@ ide_do_rw_cdrom(ide_drive_t *drive, struct request *rq, sector_t block) rq->cmd_type == REQ_TYPE_ATA_PC) { return cdrom_do_block_pc(drive, rq); } else if (blk_special_request(rq)) { - /* - * right now this can only be a reset... - */ + + /* right now this can only be a reset... */ cdrom_end_request(drive, 1); return ide_stopped; } @@ -1262,16 +1247,15 @@ ide_do_rw_cdrom(ide_drive_t *drive, struct request *rq, sector_t block) -/**************************************************************************** +/* * Ioctl handling. * - * Routines which queue packet commands take as a final argument a pointer - * to a request_sense struct. If execution of the command results - * in an error with a CHECK CONDITION status, this structure will be filled - * with the results of the subsequent request sense command. The pointer - * can also be NULL, in which case no sense information is returned. + * Routines which queue packet commands take as a final argument a pointer to a + * request_sense struct. If execution of the command results in an error with a + * CHECK CONDITION status, this structure will be filled with the results of the + * subsequent request sense command. The pointer can also be NULL, in which case + * no sense information is returned. */ - static void msf_from_bcd(struct atapi_msf *msf) { @@ -1293,8 +1277,8 @@ int cdrom_check_status(ide_drive_t *drive, struct request_sense *sense) req.cmd_flags |= REQ_QUIET; /* - * Sanyo 3 CD changer uses byte 7 of TEST_UNIT_READY to - * switch CDs instead of supporting the LOAD_UNLOAD opcode. + * Sanyo 3 CD changer uses byte 7 of TEST_UNIT_READY to switch CDs + * instead of supporting the LOAD_UNLOAD opcode. */ req.cmd[7] = cdi->sanyo_slot % 3; @@ -1370,7 +1354,7 @@ int ide_cd_read_toc(ide_drive_t *drive, struct request_sense *sense) unsigned long sectors_per_frame = SECTORS_PER_FRAME; if (toc == NULL) { - /* Try to allocate space. */ + /* try to allocate space */ toc = kmalloc(sizeof(struct atapi_toc), GFP_KERNEL); if (toc == NULL) { printk(KERN_ERR "%s: No cdrom TOC buffer!\n", drive->name); @@ -1379,27 +1363,29 @@ int ide_cd_read_toc(ide_drive_t *drive, struct request_sense *sense) info->toc = toc; } - /* Check to see if the existing data is still valid. - If it is, just return. */ + /* + * Check to see if the existing data is still valid. If it is, + * just return. + */ (void) cdrom_check_status(drive, sense); if (info->cd_flags & IDE_CD_FLAG_TOC_VALID) return 0; - /* Try to get the total cdrom capacity and sector size. */ + /* try to get the total cdrom capacity and sector size. */ stat = cdrom_read_capacity(drive, &toc->capacity, §ors_per_frame, sense); if (stat) toc->capacity = 0x1fffff; set_capacity(info->disk, toc->capacity * sectors_per_frame); - /* Save a private copy of te TOC capacity for error handling */ + /* save a private copy of the TOC capacity for error handling */ drive->probed_capacity = toc->capacity * sectors_per_frame; blk_queue_hardsect_size(drive->queue, sectors_per_frame << SECTOR_BITS); - /* First read just the header, so we know how long the TOC is. */ + /* first read just the header, so we know how long the TOC is. */ stat = cdrom_read_tocentry(drive, 0, 1, 0, (char *) &toc->hdr, sizeof(struct atapi_toc_header), sense); if (stat) @@ -1416,7 +1402,7 @@ int ide_cd_read_toc(ide_drive_t *drive, struct request_sense *sense) if (ntracks > MAX_TRACKS) ntracks = MAX_TRACKS; - /* Now read the whole schmeer. */ + /* now read the whole schmeer. */ stat = cdrom_read_tocentry(drive, toc->hdr.first_track, 1, 0, (char *)&toc->hdr, sizeof(struct atapi_toc_header) + @@ -1424,15 +1410,18 @@ int ide_cd_read_toc(ide_drive_t *drive, struct request_sense *sense) sizeof(struct atapi_toc_entry), sense); if (stat && toc->hdr.first_track > 1) { - /* Cds with CDI tracks only don't have any TOC entries, - despite of this the returned values are - first_track == last_track = number of CDI tracks + 1, - so that this case is indistinguishable from the same - layout plus an additional audio track. - If we get an error for the regular case, we assume - a CDI without additional audio tracks. In this case - the readable TOC is empty (CDI tracks are not included) - and only holds the Leadout entry. Heiko Eißfeldt */ + /* + * Cds with CDI tracks only don't have any TOC entries, despite + * of this the returned values are + * first_track == last_track = number of CDI tracks + 1, + * so that this case is indistinguishable from the same layout + * plus an additional audio track. If we get an error for the + * regular case, we assume a CDI without additional audio + * tracks. In this case the readable TOC is empty (CDI tracks + * are not included) and only holds the Leadout entry. + * + * Heiko Eißfeldt. + */ ntracks = 0; stat = cdrom_read_tocentry(drive, CDROM_LEADOUT, 1, 0, (char *)&toc->hdr, @@ -1473,9 +1462,8 @@ int ide_cd_read_toc(ide_drive_t *drive, struct request_sense *sense) toc->ent[i].addr.msf.frame); } - /* Read the multisession information. */ if (toc->hdr.first_track != CDROM_LEADOUT) { - /* Read the multisession information. */ + /* read the multisession information. */ stat = cdrom_read_tocentry(drive, 0, 0, 1, (char *)&ms_tmp, sizeof(ms_tmp), sense); if (stat) @@ -1488,7 +1476,7 @@ int ide_cd_read_toc(ide_drive_t *drive, struct request_sense *sense) } if (info->cd_flags & IDE_CD_FLAG_TOCADDR_AS_BCD) { - /* Re-read multisession information using MSF format */ + /* re-read multisession information using MSF format */ stat = cdrom_read_tocentry(drive, 0, 1, 1, (char *)&ms_tmp, sizeof(ms_tmp), sense); if (stat) @@ -1502,7 +1490,7 @@ int ide_cd_read_toc(ide_drive_t *drive, struct request_sense *sense) toc->xa_flag = (ms_tmp.hdr.first_track != ms_tmp.hdr.last_track); - /* Now try to get the total cdrom capacity. */ + /* now try to get the total cdrom capacity. */ stat = cdrom_get_last_written(cdi, &last_written); if (!stat && (last_written > toc->capacity)) { toc->capacity = last_written; @@ -1527,7 +1515,8 @@ int ide_cdrom_get_capabilities(ide_drive_t *drive, u8 *buf) size -= ATAPI_CAPABILITIES_PAGE_PAD_SIZE; init_cdrom_command(&cgc, buf, size, CGC_DATA_UNKNOWN); - do { /* we seem to get stat=0x01,err=0x00 the first time (??) */ + do { + /* we seem to get stat=0x01,err=0x00 the first time (??) */ stat = cdrom_mode_sense(cdi, &cgc, GPMODE_CAPABILITIES_PAGE, 0); if (!stat) break; @@ -1622,11 +1611,10 @@ int ide_cdrom_probe_capabilities(ide_drive_t *drive) } /* - * we have to cheat a little here. the packet will eventually - * be queued with ide_cdrom_packet(), which extracts the - * drive from cdi->handle. Since this device hasn't been - * registered with the Uniform layer yet, it can't do this. - * Same goes for cdi->ops. + * we have to cheat a little here. the packet will eventually be queued + * with ide_cdrom_packet(), which extracts the drive from cdi->handle. + * Since this device hasn't been registered with the Uniform layer yet, + * it can't do this. Same goes for cdi->ops. */ cdi->handle = drive; cdi->ops = &ide_cdrom_dops; @@ -1695,9 +1683,7 @@ int ide_cdrom_probe_capabilities(ide_drive_t *drive) return nslots; } -/* - * standard prep_rq_fn that builds 10 byte cmds - */ +/* standard prep_rq_fn that builds 10 byte cmds */ static int ide_cdrom_prep_fs(struct request_queue *q, struct request *rq) { int hard_sect = queue_hardsect_size(q); @@ -1736,9 +1722,7 @@ static int ide_cdrom_prep_pc(struct request *rq) { u8 *c = rq->cmd; - /* - * Transform 6-byte read/write commands to the 10-byte version - */ + /* Transform 6-byte read/write commands to the 10-byte version */ if (c[0] == READ_6 || c[0] == WRITE_6) { c[8] = c[4]; c[5] = c[3]; @@ -1902,13 +1886,12 @@ int ide_cdrom_setup(ide_drive_t *drive) id->fw_rev[4] == '1' && id->fw_rev[6] <= '2') cd->cd_flags |= IDE_CD_FLAG_TOCTRACKS_AS_BCD; else if (cd->cd_flags & IDE_CD_FLAG_SANYO_3CD) - cdi->sanyo_slot = 3; /* 3 => use CD in slot 0 */ + /* 3 => use CD in slot 0 */ + cdi->sanyo_slot = 3; nslots = ide_cdrom_probe_capabilities(drive); - /* - * set correct block size - */ + /* set correct block size */ blk_queue_hardsect_size(drive->queue, CD_FRAMESIZE); if (drive->autotune == IDE_TUNE_DEFAULT || @@ -2093,7 +2076,7 @@ static struct block_device_operations idecd_ops = { .revalidate_disk = idecd_revalidate_disk }; -/* options */ +/* module options */ static char *ignore; module_param(ignore, charp, 0400); -- 1.5.4.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] ide-cd: fixup comments 2008-02-27 18:20 ` [PATCH 3/5] ide-cd: fixup comments Borislav Petkov @ 2008-02-27 21:18 ` Bartlomiej Zolnierkiewicz 0 siblings, 0 replies; 17+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2008-02-27 21:18 UTC (permalink / raw) To: Borislav Petkov; +Cc: linux-kernel, linux-ide, Borislav Petkov On Wednesday 27 February 2008, Borislav Petkov wrote: > Signed-off-by: Borislav Petkov <petkovbb@gmail.com> interdiff for the merged version diff -u b/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c --- b/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -296,7 +296,7 @@ struct request *rq = HWGROUP(drive)->rq; int stat, err, sense_key; - /* check for errors. */ + /* check for errors */ stat = ide_read_status(drive); if (stat_ret) @@ -305,7 +305,7 @@ if (OK_STAT(stat, good_stat, BAD_R_STAT)) return 0; - /* get the IDE error register. */ + /* get the IDE error register */ err = ide_read_error(drive); sense_key = err >> 4; @@ -335,11 +335,11 @@ if (blk_pc_request(rq) && !rq->errors) rq->errors = SAM_STAT_CHECK_CONDITION; - /* check for tray open. */ + /* check for tray open */ if (sense_key == NOT_READY) { cdrom_saw_media_change(drive); } else if (sense_key == UNIT_ATTENTION) { - /* check for media change. */ + /* check for media change */ cdrom_saw_media_change(drive); return 0; } else if (sense_key == ILLEGAL_REQUEST && @@ -352,7 +352,7 @@ * cdrom_log_sense() knows this! */ } else if (!(rq->cmd_flags & REQ_QUIET)) { - /* otherwise, print an error. */ + /* otherwise, print an error */ ide_dump_status(drive, "packet command error", stat); } @@ -368,24 +368,24 @@ } else if (blk_fs_request(rq)) { int do_end_request = 0; - /* handle errors from READ and WRITE requests. */ + /* handle errors from READ and WRITE requests */ if (blk_noretry_request(rq)) do_end_request = 1; if (sense_key == NOT_READY) { - /* tray open. */ + /* tray open */ if (rq_data_dir(rq) == READ) { cdrom_saw_media_change(drive); - /* fail the request. */ + /* fail the request */ printk("%s: tray open\n", drive->name); do_end_request = 1; } else { struct cdrom_info *info = drive->driver_data; /* - * allow the drive 5 seconds to recover, some + * Allow the drive 5 seconds to recover, some * devices will return this error while flushing * data from cache. */ @@ -408,7 +408,7 @@ } } } else if (sense_key == UNIT_ATTENTION) { - /* media change. */ + /* media change */ cdrom_saw_media_change (drive); /* @@ -437,11 +437,11 @@ ide_dump_status_no_sense(drive, "media error (blank)", stat); do_end_request = 1; } else if ((err & ~ABRT_ERR) != 0) { - /* go to the default handler for other errors. */ + /* go to the default handler for other errors */ ide_error(drive, "cdrom_decode_status", stat); return 1; } else if ((++rq->errors > ERROR_MAX)) { - /* we've racked up too many retries, abort. */ + /* we've racked up too many retries, abort */ do_end_request = 1; } @@ -464,7 +464,7 @@ cdrom_end_request(drive, 0); } - /* retry, or handle the next request. */ + /* retry, or handle the next request */ return 1; end_request: @@ -527,7 +527,7 @@ struct cdrom_info *info = drive->driver_data; ide_hwif_t *hwif = drive->hwif; - /* wait for the controller to be idle. */ + /* wait for the controller to be idle */ if (ide_wait_stat(&startstop, drive, 0, BUSY_STAT, WAIT_READY)) return startstop; @@ -535,7 +535,7 @@ if (info->dma) info->dma = !hwif->dma_setup(drive); - /* set up the controller registers. */ + /* set up the controller registers */ ide_pktcmd_tf_load(drive, IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL | IDE_TFLAG_NO_SELECT_MASK, xferlen, info->dma); @@ -583,21 +583,21 @@ * from the device. DRQ should how be set. */ - /* check for errors. */ + /* check for errors */ if (cdrom_decode_status(drive, DRQ_STAT, NULL)) return ide_stopped; - /* ok, next interrupt will be DMA interrupt. */ + /* ok, next interrupt will be DMA interrupt */ if (info->dma) drive->waiting_for_dma = 1; } else { - /* otherwise, we must wait for DRQ to get set. */ + /* otherwise, we must wait for DRQ to get set */ if (ide_wait_stat(&startstop, drive, DRQ_STAT, BUSY_STAT, WAIT_READY)) return startstop; } - /* arm the interrupt handler. */ + /* arm the interrupt handler */ ide_set_handler(drive, handler, rq->timeout, cdrom_timer_expiry); /* ATAPI commands get padded out to 12 bytes minimum */ @@ -605,7 +605,7 @@ if (cmd_len < ATAPI_MIN_CDB_BYTES) cmd_len = ATAPI_MIN_CDB_BYTES; - /* send the command to the device. */ + /* send the command to the device */ HWIF(drive)->atapi_output_bytes(drive, rq->cmd, cmd_len); /* start the DMA if need be */ @@ -664,7 +664,7 @@ } else if (rw == 0 && ireason == 1) { /* * Some drives (ASUS) seem to tell us that status info is - * available. just get it and ignore. + * available. Just get it and ignore. */ (void)ide_read_status(drive); return 0; @@ -754,7 +754,7 @@ /* set up the command */ rq->timeout = ATAPI_WAIT_PC; - /* send the command to the drive and return. */ + /* send the command to the drive and return */ return cdrom_transfer_packet_command(drive, rq, cdrom_newpc_intr); } @@ -774,9 +774,8 @@ info->cd_flags |= IDE_CD_FLAG_SEEKING; if (retry && time_after(jiffies, info->start_seek + IDECD_SEEK_TIMER)) { - if (--retry == 0) { - drive->dsc_overlap = 0; - } + if (--retry == 0) + drive->dsc_overlap = 0; } return ide_stopped; } @@ -850,7 +849,7 @@ if (rq->sense == NULL) rq->sense = &sense; - /* start of retry loop. */ + /* start of retry loop */ do { int error; unsigned long time = jiffies; @@ -881,16 +880,16 @@ */ ssleep(2); } else { - /* otherwise, don't retry. */ + /* otherwise, don't retry */ retries = 0; } --retries; } - /* end of retry loop. */ + /* end of retry loop */ } while ((rq->cmd_flags & REQ_FAILED) && retries >= 0); - /* return an error if the command failed. */ + /* return an error if the command failed */ return (rq->cmd_flags & REQ_FAILED) ? -EIO : 0; } @@ -916,7 +915,7 @@ unsigned int timeout; u8 lowcyl, highcyl; - /* check for errors. */ + /* check for errors */ dma = info->dma; if (dma) { info->dma = 0; @@ -942,7 +941,7 @@ goto end_request; } - /* ok we fall to pio :/ */ + /* ok we fall to pio :/ */ ireason = hwif->INB(hwif->io_ports[IDE_IREASON_OFFSET]) & 0x3; lowcyl = hwif->INB(hwif->io_ports[IDE_BCOUNTL_OFFSET]); highcyl = hwif->INB(hwif->io_ports[IDE_BCOUNTH_OFFSET]); @@ -974,7 +973,7 @@ return ide_stopped; } else if (!blk_pc_request(rq)) { ide_cd_request_sense_fixup(rq); - /* complain if we still have data left to transfer. */ + /* complain if we still have data left to transfer */ uptodate = rq->data_len ? 0 : 1; } goto end_request; @@ -1155,7 +1154,7 @@ if (write) cd->devinfo.media_written = 1; - /* start sending the read/write request to the drive. */ + /* start sending the read/write request to the drive */ return cdrom_start_packet_command(drive, 32768, cdrom_start_rw_cont); } @@ -1197,7 +1196,7 @@ info->dma = 0; } - /* start sending the command to the drive. */ + /* start sending the command to the drive */ return cdrom_start_packet_command(drive, rq->data_len, cdrom_do_newpc_cont); } @@ -1234,7 +1233,6 @@ rq->cmd_type == REQ_TYPE_ATA_PC) { return cdrom_do_block_pc(drive, rq); } else if (blk_special_request(rq)) { - /* right now this can only be a reset... */ cdrom_end_request(drive, 1); return ide_stopped; @@ -1372,7 +1370,7 @@ if (info->cd_flags & IDE_CD_FLAG_TOC_VALID) return 0; - /* try to get the total cdrom capacity and sector size. */ + /* try to get the total cdrom capacity and sector size */ stat = cdrom_read_capacity(drive, &toc->capacity, §ors_per_frame, sense); if (stat) @@ -1385,7 +1383,7 @@ blk_queue_hardsect_size(drive->queue, sectors_per_frame << SECTOR_BITS); - /* first read just the header, so we know how long the TOC is. */ + /* first read just the header, so we know how long the TOC is */ stat = cdrom_read_tocentry(drive, 0, 1, 0, (char *) &toc->hdr, sizeof(struct atapi_toc_header), sense); if (stat) @@ -1402,7 +1400,7 @@ if (ntracks > MAX_TRACKS) ntracks = MAX_TRACKS; - /* now read the whole schmeer. */ + /* now read the whole schmeer */ stat = cdrom_read_tocentry(drive, toc->hdr.first_track, 1, 0, (char *)&toc->hdr, sizeof(struct atapi_toc_header) + @@ -1463,7 +1461,7 @@ } if (toc->hdr.first_track != CDROM_LEADOUT) { - /* read the multisession information. */ + /* read the multisession information */ stat = cdrom_read_tocentry(drive, 0, 0, 1, (char *)&ms_tmp, sizeof(ms_tmp), sense); if (stat) @@ -1490,7 +1488,7 @@ toc->xa_flag = (ms_tmp.hdr.first_track != ms_tmp.hdr.last_track); - /* now try to get the total cdrom capacity. */ + /* now try to get the total cdrom capacity */ stat = cdrom_get_last_written(cdi, &last_written); if (!stat && (last_written > toc->capacity)) { toc->capacity = last_written; @@ -1611,7 +1609,7 @@ } /* - * we have to cheat a little here. the packet will eventually be queued + * We have to cheat a little here. the packet will eventually be queued * with ide_cdrom_packet(), which extracts the drive from cdi->handle. * Since this device hasn't been registered with the Uniform layer yet, * it can't do this. Same goes for cdi->ops. @@ -1722,7 +1720,7 @@ { u8 *c = rq->cmd; - /* Transform 6-byte read/write commands to the 10-byte version */ + /* transform 6-byte read/write commands to the 10-byte version */ if (c[0] == READ_6 || c[0] == WRITE_6) { c[8] = c[4]; c[5] = c[3]; ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/5] ide-cd: shorten lines longer than 80 columns 2008-02-27 18:20 [PATCH 0/5] ide-cd: trivial fixes Borislav Petkov ` (2 preceding siblings ...) 2008-02-27 18:20 ` [PATCH 3/5] ide-cd: fixup comments Borislav Petkov @ 2008-02-27 18:20 ` Borislav Petkov 2008-02-27 21:18 ` Bartlomiej Zolnierkiewicz 2008-02-27 18:20 ` [PATCH 5/5] ide-cd: fix remaining checkpatch.pl issues Borislav Petkov 2008-02-27 21:18 ` [PATCH 0/5] ide-cd: trivial fixes Bartlomiej Zolnierkiewicz 5 siblings, 1 reply; 17+ messages in thread From: Borislav Petkov @ 2008-02-27 18:20 UTC (permalink / raw) To: bzolnier; +Cc: linux-kernel, linux-ide, Borislav Petkov Signed-off-by: Borislav Petkov <petkovbb@gmail.com> --- drivers/ide/ide-cd.c | 75 +++++++++++++++++++++++++++++++++---------------- 1 files changed, 50 insertions(+), 25 deletions(-) diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index bb501a4..84415cd 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -279,11 +279,11 @@ static void cdrom_end_request(ide_drive_t *drive, int uptodate) ide_end_request(drive, uptodate, nsectors); } -static void ide_dump_status_no_sense(ide_drive_t *drive, const char *msg, u8 stat) +static void ide_dump_status_no_sense(ide_drive_t *drive, const char *msg, u8 st) { - if (stat & 0x80) + if (st & 0x80) return; - ide_dump_status(drive, msg, stat); + ide_dump_status(drive, msg, st); } /* @@ -390,7 +390,8 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) * data from cache. */ if (!rq->errors) - info->write_timeout = jiffies + ATAPI_WAIT_WRITE_BUSY; + info->write_timeout = jiffies + + ATAPI_WAIT_WRITE_BUSY; rq->errors = 1; if (time_after(jiffies, info->write_timeout)) do_end_request = 1; @@ -403,7 +404,8 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) */ spin_lock_irqsave(&ide_lock, flags); blk_plug_device(drive->queue); - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&ide_lock, + flags); return 1; } } @@ -430,11 +432,14 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) * No point in re-trying a zillion times on a bad * sector. If we got here the error is not correctable. */ - ide_dump_status_no_sense(drive, "media error (bad sector)", stat); + ide_dump_status_no_sense(drive, + "media error (bad sector)", + stat); do_end_request = 1; } else if (sense_key == BLANK_CHECK) { /* disk appears blank ?? */ - ide_dump_status_no_sense(drive, "media error (blank)", stat); + ide_dump_status_no_sense(drive, "media error (blank)", + stat); do_end_request = 1; } else if ((err & ~ABRT_ERR) != 0) { /* go to the default handler for other errors. */ @@ -504,7 +509,8 @@ static int cdrom_timer_expiry(ide_drive_t *drive) break; default: if (!(rq->cmd_flags & REQ_QUIET)) - printk(KERN_INFO "ide-cd: cmd 0x%x timed out\n", rq->cmd[0]); + printk(KERN_INFO "ide-cd: cmd 0x%x timed out\n", + rq->cmd[0]); wait = 0; break; } @@ -545,7 +551,8 @@ static ide_startstop_t cdrom_start_packet_command(ide_drive_t *drive, drive->waiting_for_dma = 0; /* packet command */ - ide_execute_command(drive, WIN_PACKETCMD, handler, ATAPI_WAIT_PC, cdrom_timer_expiry); + ide_execute_command(drive, WIN_PACKETCMD, handler, + ATAPI_WAIT_PC, cdrom_timer_expiry); return ide_started; } else { unsigned long flags; @@ -802,7 +809,8 @@ static ide_startstop_t cdrom_start_seek(ide_drive_t *drive, unsigned int block) info->dma = 0; info->start_seek = jiffies; - return cdrom_start_packet_command(drive, 0, cdrom_start_seek_continuation); + return cdrom_start_packet_command(drive, 0, + cdrom_start_seek_continuation); } /* @@ -812,13 +820,15 @@ static ide_startstop_t cdrom_start_seek(ide_drive_t *drive, unsigned int block) static void restore_request(struct request *rq) { if (rq->buffer != bio_data(rq->bio)) { - sector_t n = (rq->buffer - (char *) bio_data(rq->bio)) / SECTOR_SIZE; + sector_t n = (rq->buffer - (char *) bio_data(rq->bio)) / + SECTOR_SIZE; rq->buffer = bio_data(rq->bio); rq->nr_sectors += n; rq->sector -= n; } - rq->hard_cur_sectors = rq->current_nr_sectors = bio_cur_sectors(rq->bio); + rq->current_nr_sectors = bio_cur_sectors(rq->bio); + rq->hard_cur_sectors = rq->current_nr_sectors; rq->hard_nr_sectors = rq->nr_sectors; rq->hard_sector = rq->sector; rq->q->prep_rq_fn(rq->q, rq); @@ -1183,7 +1193,8 @@ static ide_startstop_t cdrom_do_block_pc(ide_drive_t *drive, struct request *rq) /* sg request */ if (rq->bio) { int mask = drive->queue->dma_alignment; - unsigned long addr = (unsigned long) page_address(bio_page(rq->bio)); + unsigned long addr = (unsigned long) + page_address(bio_page(rq->bio)); info->dma = drive->using_dma; @@ -1198,7 +1209,8 @@ static ide_startstop_t cdrom_do_block_pc(ide_drive_t *drive, struct request *rq) } /* start sending the command to the drive. */ - return cdrom_start_packet_command(drive, rq->data_len, cdrom_do_newpc_cont); + return cdrom_start_packet_command(drive, rq->data_len, + cdrom_do_newpc_cont); } /* @@ -1217,14 +1229,21 @@ ide_do_rw_cdrom(ide_drive_t *drive, struct request *rq, sector_t block) if ((stat & SEEK_STAT) != SEEK_STAT) { if (elapsed < IDECD_SEEK_TIMEOUT) { - ide_stall_queue(drive, IDECD_SEEK_TIMER); + ide_stall_queue(drive, + IDECD_SEEK_TIMER); return ide_stopped; } - printk(KERN_ERR "%s: DSC timeout\n", drive->name); + printk(KERN_ERR "%s: DSC timeout\n", + drive->name); } info->cd_flags &= ~IDE_CD_FLAG_SEEKING; } - if ((rq_data_dir(rq) == READ) && IDE_LARGE_SEEK(info->last_block, block, IDECD_SEEK_THRESHOLD) && drive->dsc_overlap) + if ((rq_data_dir(rq) == READ) && + IDE_LARGE_SEEK(info->last_block, + block, + IDECD_SEEK_THRESHOLD) && + drive->dsc_overlap) + action = cdrom_start_seek(drive, block); else action = cdrom_start_rw(drive, rq); @@ -1357,7 +1376,8 @@ int ide_cd_read_toc(ide_drive_t *drive, struct request_sense *sense) /* try to allocate space */ toc = kmalloc(sizeof(struct atapi_toc), GFP_KERNEL); if (toc == NULL) { - printk(KERN_ERR "%s: No cdrom TOC buffer!\n", drive->name); + printk(KERN_ERR "%s: No cdrom TOC buffer!\n", + drive->name); return -ENOMEM; } info->toc = toc; @@ -1600,7 +1620,8 @@ int ide_cdrom_probe_capabilities(ide_drive_t *drive) if (drive->media == ide_optical) { cdi->mask &= ~(CDC_MO_DRIVE | CDC_RAM); - printk(KERN_ERR "%s: ATAPI magneto-optical drive\n", drive->name); + printk(KERN_ERR "%s: ATAPI magneto-optical drive\n", + drive->name); return nslots; } @@ -1899,7 +1920,8 @@ int ide_cdrom_setup(ide_drive_t *drive) drive->dsc_overlap = (drive->next != drive); if (ide_cdrom_register(drive, nslots)) { - printk(KERN_ERR "%s: ide_cdrom_setup failed to register device with the cdrom driver.\n", drive->name); + printk(KERN_ERR "%s: %s failed to register device with the" + " cdrom driver.\n", drive->name, __func__); cd->devinfo.handle = NULL; return 1; } @@ -1927,8 +1949,8 @@ static void ide_cd_release(struct kref *kref) kfree(info->toc); if (devinfo->handle == drive && unregister_cdrom(devinfo)) - printk(KERN_ERR "%s: %s failed to unregister device from the cdrom " - "driver.\n", __func__, drive->name); + printk(KERN_ERR "%s: %s failed to unregister device from the" + " cdrom driver.\n", drive->name, __func__); drive->dsc_overlap = 0; drive->driver_data = NULL; blk_queue_prep_rq(drive->queue, NULL); @@ -2097,17 +2119,20 @@ static int ide_cd_probe(ide_drive_t *drive) /* skip drives that we were told to ignore */ if (ignore != NULL) { if (strstr(ignore, drive->name)) { - printk(KERN_INFO "ide-cd: ignoring drive %s\n", drive->name); + printk(KERN_INFO "ide-cd: ignoring drive %s\n", + drive->name); goto failed; } } if (drive->scsi) { - printk(KERN_INFO "ide-cd: passing drive %s to ide-scsi emulation.\n", drive->name); + printk(KERN_INFO "ide-cd: passing drive %s to ide-scsi " + "emulation.\n", drive->name); goto failed; } info = kzalloc(sizeof(struct cdrom_info), GFP_KERNEL); if (info == NULL) { - printk(KERN_ERR "%s: Can't allocate a cdrom structure\n", drive->name); + printk(KERN_ERR "%s: Can't allocate a cdrom structure\n", + drive->name); goto failed; } -- 1.5.4.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] ide-cd: shorten lines longer than 80 columns 2008-02-27 18:20 ` [PATCH 4/5] ide-cd: shorten lines longer than 80 columns Borislav Petkov @ 2008-02-27 21:18 ` Bartlomiej Zolnierkiewicz 2008-02-27 22:15 ` Borislav Petkov 0 siblings, 1 reply; 17+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2008-02-27 21:18 UTC (permalink / raw) To: Borislav Petkov; +Cc: linux-kernel, linux-ide, Borislav Petkov On Wednesday 27 February 2008, Borislav Petkov wrote: > Signed-off-by: Borislav Petkov <petkovbb@gmail.com> interdiff for the merged version diff -u b/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c --- b/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -433,13 +433,13 @@ * sector. If we got here the error is not correctable. */ ide_dump_status_no_sense(drive, - "media error (bad sector)", - stat); + "media error (bad sector)", + stat); do_end_request = 1; } else if (sense_key == BLANK_CHECK) { /* disk appears blank ?? */ ide_dump_status_no_sense(drive, "media error (blank)", - stat); + stat); do_end_request = 1; } else if ((err & ~ABRT_ERR) != 0) { /* go to the default handler for other errors */ @@ -510,7 +510,7 @@ default: if (!(rq->cmd_flags & REQ_QUIET)) printk(KERN_INFO "ide-cd: cmd 0x%x timed out\n", - rq->cmd[0]); + rq->cmd[0]); wait = 0; break; } @@ -552,7 +552,7 @@ /* packet command */ ide_execute_command(drive, WIN_PACKETCMD, handler, - ATAPI_WAIT_PC, cdrom_timer_expiry); + ATAPI_WAIT_PC, cdrom_timer_expiry); return ide_started; } else { unsigned long flags; @@ -809,7 +809,7 @@ info->dma = 0; info->start_seek = jiffies; return cdrom_start_packet_command(drive, 0, - cdrom_start_seek_continuation); + cdrom_start_seek_continuation); } /* @@ -819,8 +819,8 @@ static void restore_request(struct request *rq) { if (rq->buffer != bio_data(rq->bio)) { - sector_t n = (rq->buffer - (char *) bio_data(rq->bio)) / - SECTOR_SIZE; + sector_t n = + (rq->buffer - (char *)bio_data(rq->bio)) / SECTOR_SIZE; rq->buffer = bio_data(rq->bio); rq->nr_sectors += n; @@ -1192,8 +1192,8 @@ /* sg request */ if (rq->bio) { int mask = drive->queue->dma_alignment; - unsigned long addr = (unsigned long) - page_address(bio_page(rq->bio)); + unsigned long addr = + (unsigned long)page_address(bio_page(rq->bio)); info->dma = drive->using_dma; @@ -1233,16 +1233,14 @@ return ide_stopped; } printk(KERN_ERR "%s: DSC timeout\n", - drive->name); + drive->name); } info->cd_flags &= ~IDE_CD_FLAG_SEEKING; } - if ((rq_data_dir(rq) == READ) && - IDE_LARGE_SEEK(info->last_block, - block, - IDECD_SEEK_THRESHOLD) && - drive->dsc_overlap) - + if (rq_data_dir(rq) == READ && + IDE_LARGE_SEEK(info->last_block, block, + IDECD_SEEK_THRESHOLD) && + drive->dsc_overlap) action = cdrom_start_seek(drive, block); else action = cdrom_start_rw(drive, rq); @@ -1375,7 +1373,7 @@ toc = kmalloc(sizeof(struct atapi_toc), GFP_KERNEL); if (toc == NULL) { printk(KERN_ERR "%s: No cdrom TOC buffer!\n", - drive->name); + drive->name); return -ENOMEM; } info->toc = toc; @@ -1619,7 +1617,7 @@ if (drive->media == ide_optical) { cdi->mask &= ~(CDC_MO_DRIVE | CDC_RAM); printk(KERN_ERR "%s: ATAPI magneto-optical drive\n", - drive->name); + drive->name); return nslots; } @@ -2118,7 +2116,7 @@ if (ignore != NULL) { if (strstr(ignore, drive->name)) { printk(KERN_INFO "ide-cd: ignoring drive %s\n", - drive->name); + drive->name); goto failed; } } @@ -2130,7 +2128,7 @@ info = kzalloc(sizeof(struct cdrom_info), GFP_KERNEL); if (info == NULL) { printk(KERN_ERR "%s: Can't allocate a cdrom structure\n", - drive->name); + drive->name); goto failed; } ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] ide-cd: shorten lines longer than 80 columns 2008-02-27 21:18 ` Bartlomiej Zolnierkiewicz @ 2008-02-27 22:15 ` Borislav Petkov 2008-02-27 23:01 ` Bartlomiej Zolnierkiewicz 0 siblings, 1 reply; 17+ messages in thread From: Borislav Petkov @ 2008-02-27 22:15 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz; +Cc: linux-kernel, linux-ide On Wed, Feb 27, 2008 at 10:18:50PM +0100, Bartlomiej Zolnierkiewicz wrote: > On Wednesday 27 February 2008, Borislav Petkov wrote: > > Signed-off-by: Borislav Petkov <petkovbb@gmail.com> > > interdiff for the merged version > > diff -u b/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c > --- b/drivers/ide/ide-cd.c > +++ b/drivers/ide/ide-cd.c > @@ -433,13 +433,13 @@ > * sector. If we got here the error is not correctable. > */ > ide_dump_status_no_sense(drive, > - "media error (bad sector)", > - stat); > + "media error (bad sector)", > + stat); > do_end_request = 1; > } else if (sense_key == BLANK_CHECK) { > /* disk appears blank ?? */ > ide_dump_status_no_sense(drive, "media error (blank)", > - stat); > + stat); > do_end_request = 1; > } else if ((err & ~ABRT_ERR) != 0) { > /* go to the default handler for other errors */ > @@ -510,7 +510,7 @@ > default: > if (!(rq->cmd_flags & REQ_QUIET)) > printk(KERN_INFO "ide-cd: cmd 0x%x timed out\n", > - rq->cmd[0]); > + rq->cmd[0]); why do you push the rq->cmd[0] thingy back here. Shouldn't it be aligned with the opening brace as the zillion others so far? > wait = 0; > break; > } > @@ -552,7 +552,7 @@ > > /* packet command */ > ide_execute_command(drive, WIN_PACKETCMD, handler, > - ATAPI_WAIT_PC, cdrom_timer_expiry); > + ATAPI_WAIT_PC, cdrom_timer_expiry); > return ide_started; > } else { > unsigned long flags; > @@ -809,7 +809,7 @@ > info->dma = 0; > info->start_seek = jiffies; > return cdrom_start_packet_command(drive, 0, > - cdrom_start_seek_continuation); > + cdrom_start_seek_continuation); > } > > /* > @@ -819,8 +819,8 @@ > static void restore_request(struct request *rq) > { > if (rq->buffer != bio_data(rq->bio)) { > - sector_t n = (rq->buffer - (char *) bio_data(rq->bio)) / > - SECTOR_SIZE; > + sector_t n = > + (rq->buffer - (char *)bio_data(rq->bio)) / SECTOR_SIZE; i must say, lines like this one always look ugly, no matter the formatting tricks. > rq->buffer = bio_data(rq->bio); > rq->nr_sectors += n; > @@ -1192,8 +1192,8 @@ > /* sg request */ > if (rq->bio) { > int mask = drive->queue->dma_alignment; > - unsigned long addr = (unsigned long) > - page_address(bio_page(rq->bio)); > + unsigned long addr = > + (unsigned long)page_address(bio_page(rq->bio)); > > info->dma = drive->using_dma; > > @@ -1233,16 +1233,14 @@ > return ide_stopped; > } > printk(KERN_ERR "%s: DSC timeout\n", > - drive->name); > + drive->name); > } > info->cd_flags &= ~IDE_CD_FLAG_SEEKING; > } > - if ((rq_data_dir(rq) == READ) && > - IDE_LARGE_SEEK(info->last_block, > - block, > - IDECD_SEEK_THRESHOLD) && > - drive->dsc_overlap) > - > + if (rq_data_dir(rq) == READ && > + IDE_LARGE_SEEK(info->last_block, block, > + IDECD_SEEK_THRESHOLD) && > + drive->dsc_overlap) > action = cdrom_start_seek(drive, block); > else > action = cdrom_start_rw(drive, rq); > @@ -1375,7 +1373,7 @@ > toc = kmalloc(sizeof(struct atapi_toc), GFP_KERNEL); > if (toc == NULL) { > printk(KERN_ERR "%s: No cdrom TOC buffer!\n", > - drive->name); > + drive->name); > return -ENOMEM; > } > info->toc = toc; > @@ -1619,7 +1617,7 @@ > if (drive->media == ide_optical) { > cdi->mask &= ~(CDC_MO_DRIVE | CDC_RAM); > printk(KERN_ERR "%s: ATAPI magneto-optical drive\n", > - drive->name); > + drive->name); > return nslots; > } > > @@ -2118,7 +2116,7 @@ > if (ignore != NULL) { > if (strstr(ignore, drive->name)) { > printk(KERN_INFO "ide-cd: ignoring drive %s\n", > - drive->name); > + drive->name); > goto failed; > } > } > @@ -2130,7 +2128,7 @@ > info = kzalloc(sizeof(struct cdrom_info), GFP_KERNEL); > if (info == NULL) { > printk(KERN_ERR "%s: Can't allocate a cdrom structure\n", > - drive->name); > + drive->name); > goto failed; > } > -- Regards/Gruß, Boris. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] ide-cd: shorten lines longer than 80 columns 2008-02-27 22:15 ` Borislav Petkov @ 2008-02-27 23:01 ` Bartlomiej Zolnierkiewicz 2008-02-27 23:15 ` Bartlomiej Zolnierkiewicz 2008-02-28 6:16 ` Borislav Petkov 0 siblings, 2 replies; 17+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2008-02-27 23:01 UTC (permalink / raw) To: petkovbb; +Cc: linux-kernel, linux-ide On Wednesday 27 February 2008, Borislav Petkov wrote: > On Wed, Feb 27, 2008 at 10:18:50PM +0100, Bartlomiej Zolnierkiewicz wrote: > > On Wednesday 27 February 2008, Borislav Petkov wrote: > > > Signed-off-by: Borislav Petkov <petkovbb@gmail.com> > > > > interdiff for the merged version > > [...] > > diff -u b/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c > > --- b/drivers/ide/ide-cd.c > > +++ b/drivers/ide/ide-cd.c > > @@ -510,7 +510,7 @@ > > default: > > if (!(rq->cmd_flags & REQ_QUIET)) > > printk(KERN_INFO "ide-cd: cmd 0x%x timed out\n", > > - rq->cmd[0]); > > + rq->cmd[0]); > > why do you push the rq->cmd[0] thingy back here. Shouldn't it be aligned > with the opening brace as the zillion others so far? IDE is quite consistent with the above style but maybe it is just me being used to it. I could revert this change if you feel strong about it... > > @@ -819,8 +819,8 @@ > > static void restore_request(struct request *rq) > > { > > if (rq->buffer != bio_data(rq->bio)) { > > - sector_t n = (rq->buffer - (char *) bio_data(rq->bio)) / > > - SECTOR_SIZE; > > + sector_t n = > > + (rq->buffer - (char *)bio_data(rq->bio)) / SECTOR_SIZE; > > i must say, lines like this one always look ugly, no matter the formatting > tricks. For this case there is a simple solution: fix ide-cd to use hwif->sg_table instead of having it to walk rq->bio's and the whole function will vanish. ;-) [ IOW We are putting way too much time into these coding style fixes and as the old code is replaced with the new one the coding style improves itself. ] Thanks, Bart ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] ide-cd: shorten lines longer than 80 columns 2008-02-27 23:01 ` Bartlomiej Zolnierkiewicz @ 2008-02-27 23:15 ` Bartlomiej Zolnierkiewicz 2008-02-28 6:16 ` Borislav Petkov 1 sibling, 0 replies; 17+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2008-02-27 23:15 UTC (permalink / raw) To: petkovbb; +Cc: linux-kernel, linux-ide On Thursday 28 February 2008, Bartlomiej Zolnierkiewicz wrote: [...] > [ IOW We are putting way too much time into these coding style fixes and as > the old code is replaced with the new one the coding style improves itself. ] Just to clarify: by "these" I meant only ide-cd ones, i.e. we are beautifying comments for code which is going to be heavily modified soon because of other changes like ATAPI handling unification... ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] ide-cd: shorten lines longer than 80 columns 2008-02-27 23:01 ` Bartlomiej Zolnierkiewicz 2008-02-27 23:15 ` Bartlomiej Zolnierkiewicz @ 2008-02-28 6:16 ` Borislav Petkov 1 sibling, 0 replies; 17+ messages in thread From: Borislav Petkov @ 2008-02-28 6:16 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz; +Cc: linux-kernel, linux-ide On Thu, Feb 28, 2008 at 12:01:43AM +0100, Bartlomiej Zolnierkiewicz wrote: > On Wednesday 27 February 2008, Borislav Petkov wrote: > > On Wed, Feb 27, 2008 at 10:18:50PM +0100, Bartlomiej Zolnierkiewicz wrote: > > > On Wednesday 27 February 2008, Borislav Petkov wrote: > > > > Signed-off-by: Borislav Petkov <petkovbb@gmail.com> > > > > > > interdiff for the merged version > > > > > [...] > > > > diff -u b/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c > > > --- b/drivers/ide/ide-cd.c > > > +++ b/drivers/ide/ide-cd.c > > > @@ -510,7 +510,7 @@ > > > default: > > > if (!(rq->cmd_flags & REQ_QUIET)) > > > printk(KERN_INFO "ide-cd: cmd 0x%x timed out\n", > > > - rq->cmd[0]); > > > + rq->cmd[0]); > > > > why do you push the rq->cmd[0] thingy back here. Shouldn't it be aligned > > with the opening brace as the zillion others so far? > > IDE is quite consistent with the above style but maybe it is just me being > used to it. I could revert this change if you feel strong about it... > not at all, i'm not that concerned with codestyle issues as long as it is mostly readable :). The only issue of importance is consistency along all the source files so i'll try to keep up with the rest of ide. > > > @@ -819,8 +819,8 @@ > > > static void restore_request(struct request *rq) > > > { > > > if (rq->buffer != bio_data(rq->bio)) { > > > - sector_t n = (rq->buffer - (char *) bio_data(rq->bio)) / > > > - SECTOR_SIZE; > > > + sector_t n = > > > + (rq->buffer - (char *)bio_data(rq->bio)) / SECTOR_SIZE; > > > > i must say, lines like this one always look ugly, no matter the formatting > > tricks. > > For this case there is a simple solution: fix ide-cd to use hwif->sg_table > instead of having it to walk rq->bio's and the whole function will vanish. ;-) Is this hwif->sg_table walk something similar to what it is being done in ide_build_sglist() or ide_build_dmatable()? > [ IOW We are putting way too much time into these coding style fixes and as > the old code is replaced with the new one the coding style improves itself. ] Exactly my thoughts. On the one hand, it is important to clean up the code before you stare at it for a magnitude of hours looking for bugs and actually this cleanup was supposed to be really brief, to begin with, but you know how things really work out in reality :). On the other hand, there are places in ide-cd that are just horrible and need rewriting, besides API changes. On a different note, i'm working on ripping out pipelining from ide-tape since this'll make ATAPI unification a lot easier. Do you happen to have figured out what this tape->merge_stage is supposed to do, i know it is a pipeline stage and in all r/w requests the tape->merge_stage->bh is the buffer that receives/sends the data to/from the device but still it looks really strange. What i've done so far is remove the pipeline control algos and have kept the tape->merge_stage for simplicity. In a different cycle i'll have to kick it out too replacing it with a simple data buffer. Any ideas? Thanks. -- Regards/Gruß, Boris. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/5] ide-cd: fix remaining checkpatch.pl issues 2008-02-27 18:20 [PATCH 0/5] ide-cd: trivial fixes Borislav Petkov ` (3 preceding siblings ...) 2008-02-27 18:20 ` [PATCH 4/5] ide-cd: shorten lines longer than 80 columns Borislav Petkov @ 2008-02-27 18:20 ` Borislav Petkov 2008-02-27 21:18 ` Bartlomiej Zolnierkiewicz 2008-02-27 21:18 ` [PATCH 0/5] ide-cd: trivial fixes Bartlomiej Zolnierkiewicz 5 siblings, 1 reply; 17+ messages in thread From: Borislav Petkov @ 2008-02-27 18:20 UTC (permalink / raw) To: bzolnier; +Cc: linux-kernel, linux-ide, Borislav Petkov Some of them are: WARNING: braces {} are not necessary for single statement blocks CHECK: multiple assignments should be avoided WARNING: printk() should include KERN_ facility level WARNING: no space between function name and open parenthesis '(' Signed-off-by: Borislav Petkov <petkovbb@gmail.com> --- drivers/ide/ide-cd.c | 46 ++++++++++++++++++++++------------------------ 1 files changed, 22 insertions(+), 24 deletions(-) diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index 84415cd..30cf097 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -136,8 +136,7 @@ static int cdrom_log_sense(ide_drive_t *drive, struct request *rq, return log; } -static -void cdrom_analyze_sense_data(ide_drive_t *drive, +static void cdrom_analyze_sense_data(ide_drive_t *drive, struct request *failed_command, struct request_sense *sense) { @@ -186,9 +185,9 @@ void cdrom_analyze_sense_data(ide_drive_t *drive, if (valid < 0) valid = 0; if (sector < get_capacity(info->disk) && - drive->probed_capacity - sector < 4 * 75) { + drive->probed_capacity - sector < 4 * 75) + set_capacity(info->disk, sector); - } } } @@ -219,7 +218,8 @@ static void cdrom_queue_request_sense(ide_drive_t *drive, void *sense, rq->data = sense; rq->cmd[0] = GPCMD_REQUEST_SENSE; - rq->cmd[4] = rq->data_len = 18; + rq->cmd[4] = 18; + rq->data_len = 18; rq->cmd_type = REQ_TYPE_SENSE; @@ -310,7 +310,8 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) sense_key = err >> 4; if (rq == NULL) { - printk("%s: missing rq in cdrom_decode_status\n", drive->name); + printk(KERN_ERR "%s: missing rq in cdrom_decode_status\n", + drive->name); return 1; } @@ -379,7 +380,7 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) cdrom_saw_media_change(drive); /* fail the request. */ - printk("%s: tray open\n", drive->name); + printk(KERN_ERR "%s: tray open\n", drive->name); do_end_request = 1; } else { struct cdrom_info *info = drive->driver_data; @@ -411,7 +412,7 @@ static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret) } } else if (sense_key == UNIT_ATTENTION) { /* media change. */ - cdrom_saw_media_change (drive); + cdrom_saw_media_change(drive); /* * Arrange to retry the request but be sure to give up @@ -780,11 +781,10 @@ static ide_startstop_t cdrom_seek_intr(ide_drive_t *drive) info->cd_flags |= IDE_CD_FLAG_SEEKING; - if (retry && time_after(jiffies, info->start_seek + IDECD_SEEK_TIMER)) { - if (--retry == 0) { - drive->dsc_overlap = 0; - } - } + if (retry && time_after(jiffies, info->start_seek + IDECD_SEEK_TIMER)) + if (--retry == 0) + drive->dsc_overlap = 0; + return ide_stopped; } @@ -1216,8 +1216,8 @@ static ide_startstop_t cdrom_do_block_pc(ide_drive_t *drive, struct request *rq) /* * cdrom driver request routine. */ -static ide_startstop_t -ide_do_rw_cdrom(ide_drive_t *drive, struct request *rq, sector_t block) +static ide_startstop_t ide_do_rw_cdrom(ide_drive_t *drive, struct request *rq, + sector_t block) { ide_startstop_t action; struct cdrom_info *info = drive->driver_data; @@ -1275,8 +1275,7 @@ ide_do_rw_cdrom(ide_drive_t *drive, struct request *rq, sector_t block) * subsequent request sense command. The pointer can also be NULL, in which case * no sense information is returned. */ -static -void msf_from_bcd(struct atapi_msf *msf) +static void msf_from_bcd(struct atapi_msf *msf) { msf->minute = BCD2BIN(msf->minute); msf->second = BCD2BIN(msf->second); @@ -1491,7 +1490,8 @@ int ide_cd_read_toc(ide_drive_t *drive, struct request_sense *sense) toc->last_session_lba = be32_to_cpu(ms_tmp.ent.addr.lba); } else { - ms_tmp.hdr.first_track = ms_tmp.hdr.last_track = CDROM_LEADOUT; + ms_tmp.hdr.last_track = CDROM_LEADOUT; + ms_tmp.hdr.first_track = ms_tmp.hdr.last_track; toc->last_session_lba = msf_to_lba(0, 2, 0); /* 0m 2s 0f */ } @@ -1605,8 +1605,7 @@ static int ide_cdrom_register(ide_drive_t *drive, int nslots) return register_cdrom(devinfo); } -static -int ide_cdrom_probe_capabilities(ide_drive_t *drive) +static int ide_cdrom_probe_capabilities(ide_drive_t *drive) { struct cdrom_info *cd = drive->driver_data; struct cdrom_device_info *cdi = &cd->devinfo; @@ -1796,8 +1795,8 @@ static sector_t ide_cdrom_capacity(ide_drive_t *drive) return capacity * sectors_per_frame; } -static int proc_idecd_read_capacity - (char *page, char **start, off_t off, int count, int *eof, void *data) +static int proc_idecd_read_capacity(char *page, char **start, off_t off, + int count, int *eof, void *data) { ide_drive_t *drive = data; int len; @@ -1877,8 +1876,7 @@ static unsigned int ide_cd_flags(struct hd_driveid *id) return 0; } -static -int ide_cdrom_setup(ide_drive_t *drive) +static int ide_cdrom_setup(ide_drive_t *drive) { struct cdrom_info *cd = drive->driver_data; struct cdrom_device_info *cdi = &cd->devinfo; -- 1.5.4.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/5] ide-cd: fix remaining checkpatch.pl issues 2008-02-27 18:20 ` [PATCH 5/5] ide-cd: fix remaining checkpatch.pl issues Borislav Petkov @ 2008-02-27 21:18 ` Bartlomiej Zolnierkiewicz 0 siblings, 0 replies; 17+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2008-02-27 21:18 UTC (permalink / raw) To: Borislav Petkov; +Cc: linux-kernel, linux-ide, Borislav Petkov On Wednesday 27 February 2008, Borislav Petkov wrote: > Some of them are: > WARNING: braces {} are not necessary for single statement blocks > CHECK: multiple assignments should be avoided > WARNING: printk() should include KERN_ facility level > WARNING: no space between function name and open parenthesis '(' > > Signed-off-by: Borislav Petkov <petkovbb@gmail.com> interdiff for the merged version diff -u b/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c --- b/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -185,8 +185,7 @@ if (valid < 0) valid = 0; if (sector < get_capacity(info->disk) && - drive->probed_capacity - sector < 4 * 75) - + drive->probed_capacity - sector < 4 * 75) set_capacity(info->disk, sector); } } @@ -218,7 +217,7 @@ rq->data = sense; rq->cmd[0] = GPCMD_REQUEST_SENSE; - rq->cmd[4] = 18; + rq->cmd[4] = 18; rq->data_len = 18; rq->cmd_type = REQ_TYPE_SENSE; @@ -310,8 +309,8 @@ sense_key = err >> 4; if (rq == NULL) { - printk(KERN_ERR "%s: missing rq in cdrom_decode_status\n", - drive->name); + printk(KERN_ERR "%s: missing rq in %s\n", + drive->name, __func__); return 1; } ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] ide-cd: trivial fixes 2008-02-27 18:20 [PATCH 0/5] ide-cd: trivial fixes Borislav Petkov ` (4 preceding siblings ...) 2008-02-27 18:20 ` [PATCH 5/5] ide-cd: fix remaining checkpatch.pl issues Borislav Petkov @ 2008-02-27 21:18 ` Bartlomiej Zolnierkiewicz 2008-02-27 22:18 ` Borislav Petkov 5 siblings, 1 reply; 17+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2008-02-27 21:18 UTC (permalink / raw) To: Borislav Petkov; +Cc: linux-kernel, linux-ide, Borislav Petkov Hi, On Wednesday 27 February 2008, Borislav Petkov wrote: > Hi Bart, > > here are the ide-cd trivial fixes split in an easier to digest format. I > cannot do the md5sum check here since all of the patches change at least one > char but i hope that this'll be redundant here since they're trivially easy > to review :) now. > > drivers/ide/ide-cd.c | 591 +++++++++++++++++++++++++------------------------- > 1 files changed, 298 insertions(+), 293 deletions(-) applied everything BTW patch #3/5 which accounts for 2/3 of the diffstat passes md5sum check here ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] ide-cd: trivial fixes 2008-02-27 21:18 ` [PATCH 0/5] ide-cd: trivial fixes Bartlomiej Zolnierkiewicz @ 2008-02-27 22:18 ` Borislav Petkov 2008-02-27 22:48 ` Bartlomiej Zolnierkiewicz 0 siblings, 1 reply; 17+ messages in thread From: Borislav Petkov @ 2008-02-27 22:18 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz; +Cc: linux-kernel, linux-ide On Wed, Feb 27, 2008 at 10:18:40PM +0100, Bartlomiej Zolnierkiewicz wrote: > > Hi, > > On Wednesday 27 February 2008, Borislav Petkov wrote: > > Hi Bart, > > > > here are the ide-cd trivial fixes split in an easier to digest format. I > > cannot do the md5sum check here since all of the patches change at least one > > char but i hope that this'll be redundant here since they're trivially easy > > to review :) now. > > > > drivers/ide/ide-cd.c | 591 +++++++++++++++++++++++++------------------------- > > 1 files changed, 298 insertions(+), 293 deletions(-) > > applied everything > > BTW patch #3/5 which accounts for 2/3 of the diffstat passes md5sum check here huh, are you sure? The fixup-comments patch adds at least one char to the source file so the md5sums cannot be the same. -- Regards/Gruß, Boris. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] ide-cd: trivial fixes 2008-02-27 22:18 ` Borislav Petkov @ 2008-02-27 22:48 ` Bartlomiej Zolnierkiewicz 2008-02-28 5:52 ` Borislav Petkov 0 siblings, 1 reply; 17+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2008-02-27 22:48 UTC (permalink / raw) To: petkovbb; +Cc: linux-kernel, linux-ide On Wednesday 27 February 2008, Borislav Petkov wrote: > On Wed, Feb 27, 2008 at 10:18:40PM +0100, Bartlomiej Zolnierkiewicz wrote: > > > > Hi, > > > > On Wednesday 27 February 2008, Borislav Petkov wrote: > > > Hi Bart, > > > > > > here are the ide-cd trivial fixes split in an easier to digest format. I > > > cannot do the md5sum check here since all of the patches change at least one > > > char but i hope that this'll be redundant here since they're trivially easy > > > to review :) now. > > > > > > drivers/ide/ide-cd.c | 591 +++++++++++++++++++++++++------------------------- > > > 1 files changed, 298 insertions(+), 293 deletions(-) > > > > applied everything > > > > BTW patch #3/5 which accounts for 2/3 of the diffstat passes md5sum check here > > huh, are you sure? The fixup-comments patch adds at least one char to the source > file so the md5sums cannot be the same. OTOH md5sums of the resulting _binary_ files matches... ;-) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] ide-cd: trivial fixes 2008-02-27 22:48 ` Bartlomiej Zolnierkiewicz @ 2008-02-28 5:52 ` Borislav Petkov 0 siblings, 0 replies; 17+ messages in thread From: Borislav Petkov @ 2008-02-28 5:52 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz; +Cc: linux-kernel, linux-ide On Wed, Feb 27, 2008 at 11:48:34PM +0100, Bartlomiej Zolnierkiewicz wrote: > On Wednesday 27 February 2008, Borislav Petkov wrote: > > On Wed, Feb 27, 2008 at 10:18:40PM +0100, Bartlomiej Zolnierkiewicz wrote: > > > > > > Hi, > > > > > > On Wednesday 27 February 2008, Borislav Petkov wrote: > > > > Hi Bart, > > > > > > > > here are the ide-cd trivial fixes split in an easier to digest format. I > > > > cannot do the md5sum check here since all of the patches change at least one > > > > char but i hope that this'll be redundant here since they're trivially easy > > > > to review :) now. > > > > > > > > drivers/ide/ide-cd.c | 591 +++++++++++++++++++++++++------------------------- > > > > 1 files changed, 298 insertions(+), 293 deletions(-) > > > > > > applied everything > > > > > > BTW patch #3/5 which accounts for 2/3 of the diffstat passes md5sum check here > > > > huh, are you sure? The fixup-comments patch adds at least one char to the source > > file so the md5sums cannot be the same. > > OTOH md5sums of the resulting _binary_ files matches... ;-) right, the compiler discards all comments during compilation and the sole code remains unchanged. -- Regards/Gruß, Boris. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2008-02-28 6:17 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-02-27 18:20 [PATCH 0/5] ide-cd: trivial fixes Borislav Petkov 2008-02-27 18:20 ` [PATCH 1/5] ide-cd: include proper headers Borislav Petkov 2008-02-27 18:20 ` [PATCH 2/5] ide-cd: put all proc-related code at one place Borislav Petkov 2008-02-27 18:20 ` [PATCH 3/5] ide-cd: fixup comments Borislav Petkov 2008-02-27 21:18 ` Bartlomiej Zolnierkiewicz 2008-02-27 18:20 ` [PATCH 4/5] ide-cd: shorten lines longer than 80 columns Borislav Petkov 2008-02-27 21:18 ` Bartlomiej Zolnierkiewicz 2008-02-27 22:15 ` Borislav Petkov 2008-02-27 23:01 ` Bartlomiej Zolnierkiewicz 2008-02-27 23:15 ` Bartlomiej Zolnierkiewicz 2008-02-28 6:16 ` Borislav Petkov 2008-02-27 18:20 ` [PATCH 5/5] ide-cd: fix remaining checkpatch.pl issues Borislav Petkov 2008-02-27 21:18 ` Bartlomiej Zolnierkiewicz 2008-02-27 21:18 ` [PATCH 0/5] ide-cd: trivial fixes Bartlomiej Zolnierkiewicz 2008-02-27 22:18 ` Borislav Petkov 2008-02-27 22:48 ` Bartlomiej Zolnierkiewicz 2008-02-28 5:52 ` Borislav Petkov
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).