Hi Bart,

here are the pending ide-tape patches reworked which incorporate all review
points raised so far. Several new patches are appended to the original series
which i thought would be reasonable to sumbit along with the others. Also,
i've applied "ide-tape: dump gcw fields on error in idetape_identify_device()"
which is #12 and which you can simply ignore. Furthermore, #32 from the original
series got split up into the different logical changes it dealt with, as you
requested.


 Documentation/feature-removal-schedule.txt |   14 +-
 drivers/ide/ide-tape.c                     | 2764 +++++++++++++---------------
 2 files changed, 1325 insertions(+), 1453 deletions(-)
--

[ message continues ]

There should be no functional changes resulting from this patch.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   49 +++++++++++++++++------------------------------
 1 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 442d71c..c8c57ab 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -571,24 +571,6 @@ struct idetape_id_gcw {
 	unsigned protocol		:2;	/* Protocol type */
 };
 
-/*
- *	READ POSITION packet command - Data Format (From Table 6-57)
- */
-typedef struct {
-	unsigned	reserved0_10	:2;	/* Reserved */
-	unsigned	bpu		:1;	/* Block Position Unknown */	
-	unsigned	reserved0_543	:3;	/* Reserved */
-	unsigned	eop		:1;	/* End Of Partition */
-	unsigned	bop		:1;	/* Beginning Of Partition */
-	u8		partition;		/* Partition Number */
-	u8		reserved2, reserved3;	/* Reserved */
-	u32		first_block;		/* First Block Location */
-	u32		last_block;		/* Last Block Location (Optional) */
-	u8		reserved12;		/* Reserved */
-	u8		blocks_in_buffer[3];	/* Blocks In Buffer - (Optional) */
-	u32		bytes_in_buffer;	/* Bytes In Buffer (Optional) */
-} idetape_read_position_result_t;
-
 /* Structures related to the SELECT SENSE / MODE SENSE packet commands. */
 #define IDETAPE_BLOCK_DESCRIPTOR	0
 #define	IDETAPE_CAPABILITIES_PAGE	0x2a
@@ -1999,30 +1981,35 @@ static void idetape_wait_for_request (ide_drive_t *drive, struct request *rq)
 	spin_lock_irq(&tape->spinlock);
 }
 
-static ide_startstop_t idetape_read_position_callback (ide_drive_t *drive)
+static ide_startstop_t idetape_read_position_callback(ide_drive_t *drive)
 {
 	idetape_tape_t *tape = drive->driver_data;
-	idetape_read_position_result_t *result;
+	u8 *readpos = tape->pc->buffer;
 
 	debug_log(DBG_PROCS, "Enter %s\n", __func__);
 
 	if (!tape->pc->error) {
-		result = (idetape_read_position_result_t *) tape->pc->buffer;
-		debug_log(DBG_SENSE, "BOP - %s\n", result->bop ? "Yes" : ...
[ message continues ]

I removed needless "!!" while merging the patch.
--

[ message continues ]

tape->speed_control is set to 1 in idetape_setup(), but, in calculate_speeds()
its value is tested for being 0, 1, or 2. Remove the if-branches where
tape->speed_control != 1 since they are never executed. Also, rename
calculate_speeds() by adding driver's prefix as is with the other function names.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index c8c57ab..b15dd17 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -87,7 +87,8 @@ enum {
  *	the optimum value or until we reach MAX.
  *
  *	Setting the following parameter to 0 is illegal: the pipelined mode
- *	cannot be disabled (calculate_speeds() divides by tape->max_stages.)
+ *	cannot be disabled (idetape_calculate_speeds() divides by
+ *	tape->max_stages.)
  */
 #define IDETAPE_MIN_PIPELINE_STAGES	  1
 #define IDETAPE_MAX_PIPELINE_STAGES	400
@@ -1475,10 +1476,9 @@ static void idetape_create_mode_sense_cmd (idetape_pc_t *pc, u8 page_code)
 	pc->callback = &idetape_pc_callback;
 }
 
-static void calculate_speeds(ide_drive_t *drive)
+static void idetape_calculate_speeds(ide_drive_t *drive)
 {
 	idetape_tape_t *tape = drive->driver_data;
-	int full = 125, empty = 75;
 
 	if (time_after(jiffies, tape->controlled_pipeline_head_time + 120 * HZ)) {
 		tape->controlled_previous_pipeline_head = tape->controlled_last_pipeline_head;
@@ -1505,22 +1505,20 @@ static void calculate_speeds(ide_drive_t *drive)
 		}
 	}
 	tape->pipeline_head_speed = max(tape->uncontrolled_pipeline_head_speed, tape->controlled_pipeline_head_speed);
-	if (tape->speed_control == 0) {
-		tape->max_insert_speed = 5000;
-	} else if (tape->speed_control == 1) {
+
+	if (tape->speed_control == 1) {
 		if (tape->nr_pending_stages >= tape->max_stages / 2)
 			tape->max_insert_speed = tape->pipeline_head_speed +
 				(1100 - tape->pipeline_head_speed) * 2 * ...
[ message continues ]

Teach the debug logging macro to differentiate between log levels based on the
type of debug level enabled specifically instead of a threshold-based one.
Thus, convert tape->debug_level to a bitmask that is written to over /proc.

Also,
- cleanup and simplify the debug macro thus removing a lot of code lines,
- get rid of unused debug levels,
- adjust the loglevel at several places where it was simply missing (e.g.
  idetape_chrdev_open())
- move the tape ptr initialization up in idetape_chrdev_open() so that we can
  use it in the debug_log macro earlier in the function.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |  344 +++++++++++++++++-------------------------------
 1 files changed, 122 insertions(+), 222 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 4168a06..442d71c 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -45,6 +45,32 @@
 #include <asm/unaligned.h>
 #include <linux/mtio.h>
 
+enum {
+	/* output errors only */
+	DBG_ERR =		(1 << 0),
+	/* output all sense key/asc */
+	DBG_SENSE =		(1 << 1),
+	/* info regarding all chrdev-related procedures */
+	DBG_CHRDEV =		(1 << 2),
+	/* all remaining procedures */
+	DBG_PROCS =		(1 << 3),
+	/* buffer alloc info (pc_stack & rq_stack) */
+	DBG_PCRQ_STACK =	(1 << 4),
+};
+
+/* define to see debug info */
+#define IDETAPE_DEBUG_LOG		0
+
+#if IDETAPE_DEBUG_LOG
+#define debug_log(lvl, fmt, args...)			\
+{							\
+	if (tape->debug_mask & lvl)			\
+	printk(KERN_INFO "ide-tape: " fmt, ## args);	\
+}
+#else
+#define debug_log(lvl, fmt, args...) do {} while (0)
+#endif
+
 /**************************** Tunable parameters *****************************/
 
 
@@ -68,23 +94,6 @@
 #define IDETAPE_INCREASE_STAGES_RATE	 20
 
 /*
- *	The following are used to debug the driver:
- *
- *	Setting IDETAPE_DEBUG_LOG to 1 will log driver flow control.s
- *
- *	Setting them to 0 will restore normal operation mode:
- *
- *		1.	Disable ...
[ message continues ]

.. and replace it with plain enums.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   62 ++++++++++++++++++++++++-----------------------
 1 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 2857965..e0e8184 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -184,11 +184,13 @@ enum {
 /*
  *	For general magnetic tape device compatibility.
  */
-typedef enum {
-	idetape_direction_none,
-	idetape_direction_read,
-	idetape_direction_write
-} idetape_chrdev_direction_t;
+
+/* tape directions */
+enum {
+	IDETAPE_DIR_NONE  = (1 << 0),
+	IDETAPE_DIR_READ  = (1 << 1),
+	IDETAPE_DIR_WRITE = (1 << 2),
+};
 
 struct idetape_bh {
 	u32 b_size;
@@ -324,7 +326,7 @@ typedef struct ide_tape_obj {
 	/* device name */
 	char name[4];
 	/* Current character device data transfer direction */
-	idetape_chrdev_direction_t chrdev_direction;
+	u8 chrdev_dir;
 
 	/*
 	 *	Device information
@@ -1916,7 +1918,7 @@ static void idetape_init_merge_stage (idetape_tape_t *tape)
 	struct idetape_bh *bh = tape->merge_stage->bh;
 	
 	tape->bh = bh;
-	if (tape->chrdev_direction == idetape_direction_write)
+	if (tape->chrdev_dir == IDETAPE_DIR_WRITE)
 		atomic_set(&bh->b_count, 0);
 	else {
 		tape->b_data = bh->b_data;
@@ -2187,7 +2189,7 @@ static int __idetape_discard_read_pipeline (ide_drive_t *drive)
 	unsigned long flags;
 	int cnt;
 
-	if (tape->chrdev_direction != idetape_direction_read)
+	if (tape->chrdev_dir != IDETAPE_DIR_READ)
 		return 0;
 
 	/* Remove merge stage. */
@@ -2202,7 +2204,7 @@ static int __idetape_discard_read_pipeline (ide_drive_t *drive)
 
 	/* Clear pipeline flags. */
 	clear_bit(IDETAPE_PIPELINE_ERROR, &tape->flags);
-	tape->chrdev_direction = idetape_direction_none;
+	tape->chrdev_dir = IDETAPE_DIR_NONE;
 
 	/* Remove pipeline stages. */
 	if (tape->first_stage == NULL)
@@ -2242,7 +2244,7 @@ static int idetape_position_tape ...
[ message continues ]

These buffers were always statically allocated during driver initialization no
matter what. Remove them by allocating GFP_ATOMIC memory on demand. In the case
of allocation error, we only issue error msg in the *alloc_{pc,rq} thus postponing
the final error handling and cleanup in their callers. Packet command and
request memory is freed in idetape_end_request() which is at the end of the
request path entered from all callbacks. While at it, integrate comments above
member definitions in ide_tape_obj.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |  146 +++++++++++++++++++++---------------------------
 1 files changed, 63 insertions(+), 83 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 0b5ccce..b4944ef 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -54,8 +54,6 @@ enum {
 	DBG_CHRDEV =		(1 << 2),
 	/* all remaining procedures */
 	DBG_PROCS =		(1 << 3),
-	/* buffer alloc info (pc_stack & rq_stack) */
-	DBG_PCRQ_STACK =	(1 << 4),
 };
 
 /* define to see debug info */
@@ -110,13 +108,6 @@ enum {
 #define IDETAPE_PC_BUFFER_SIZE		256
 
 /*
- *	In various places in the driver, we need to allocate storage
- *	for packet commands and requests, which will remain valid while
- *	we leave the driver to wait for an interrupt or a timeout event.
- */
-#define IDETAPE_PC_STACK		(10 + IDETAPE_MAX_PC_RETRIES)
-
-/*
  * Some drives (for example, Seagate STT3401A Travan) require a very long
  * timeout, because they don't return an interrupt or clear their busy bit
  * until after the command completes (even retension commands).
@@ -255,32 +246,15 @@ typedef struct ide_tape_obj {
 	struct gendisk	*disk;
 	struct kref	kref;
 
+	/* current processed packet command */
+	idetape_pc_t *pc;
 	/*
-	 *	Since a typical character device operation requires more
-	 *	than one packet command, we provide here enough memory
-	 *	for the maximum of interconnected packet commands.
-	 *	The packet commands ...
[ message continues ]

goes before "ide-tape: remove IDETAPE_DEBUG_INFO" patch in IDE quilt tree

Cc: Borislav Petkov <bbpetkov@yahoo.de>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-tape.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 1c4f94c..712c5df 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -3402,16 +3402,17 @@ static int idetape_identify_device (ide_drive_t *drive)
 
 	/* Check that we can support this device */
 
-	if (gcw.protocol !=2 )
-		printk(KERN_ERR "ide-tape: Protocol is not ATAPI\n");
+	if (gcw.protocol != 2)
+		printk(KERN_ERR "ide-tape: Protocol (0x%02x) is not ATAPI\n",
+				gcw.protocol);
 	else if (gcw.device_type != 1)
-		printk(KERN_ERR "ide-tape: Device type is not set to tape\n");
+		printk(KERN_ERR "ide-tape: Device type (0x%02x) is not set "
+				"to tape\n", gcw.device_type);
 	else if (!gcw.removable)
 		printk(KERN_ERR "ide-tape: The removable flag is not set\n");
 	else if (gcw.packet_size != 0) {
-		printk(KERN_ERR "ide-tape: Packet size is not 12 bytes long\n");
-		if (gcw.packet_size == 1)
-			printk(KERN_ERR "ide-tape: Sorry, padding to 16 bytes is still not supported\n");
+		printk(KERN_ERR "ide-tape: Packet size (0x%02x) is not 12 "
+				"bytes long\n", gcw.packet_size);
 	} else
 		return 1;
 	return 0;
-- 
1.5.3.7

--

[ message continues ]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index a80f8d9..342ec50 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1468,7 +1468,8 @@ static void idetape_create_read_cmd(idetape_tape_t *tape, idetape_pc_t *pc, unsi
 		pc->flags |= PC_FL_DMA_RECOMMENDED;
 }
 
-static void idetape_create_read_buffer_cmd(idetape_tape_t *tape, idetape_pc_t *pc, unsigned int length, struct idetape_bh *bh)
+static void idetape_create_read_buffer_cmd(idetape_tape_t *tape,
+		idetape_pc_t *pc, struct idetape_bh *bh)
 {
 	int size = 32768;
 	struct idetape_bh *p = bh;
@@ -1486,7 +1487,8 @@ static void idetape_create_read_buffer_cmd(idetape_tape_t *tape, idetape_pc_t *p
 		atomic_set(&p->b_count, 0);
 		p = p->b_reqnext;
 	}
-	pc->request_transfer = pc->buffer_size = size;
+	pc->request_transfer = size;
+	pc->buffer_size = size;
 }
 
 static void idetape_create_write_cmd(idetape_tape_t *tape, idetape_pc_t *pc, unsigned int length, struct idetape_bh *bh)
@@ -1607,7 +1609,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 		if (!pc)
 			goto err;
 
-		idetape_create_read_buffer_cmd(tape, pc, rq->current_nr_sectors,
+		idetape_create_read_buffer_cmd(tape, pc,
 				(struct idetape_bh *)rq->special);
 		goto out;
 	}
-- 
1.5.3.7

--

[ message continues ]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   58 ++++++++++++++++++++---------------------------
 1 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 712c5df..175d507 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -520,20 +520,6 @@ enum {
 #define	IDETAPE_ERROR_FILEMARK		102
 #define	IDETAPE_ERROR_EOD		103
 
-/*
- *	The following is used to format the general configuration word of
- *	the ATAPI IDENTIFY DEVICE command.
- */
-struct idetape_id_gcw {	
-	unsigned packet_size		:2;	/* Packet Size */
-	unsigned reserved234		:3;	/* Reserved */
-	unsigned drq_type		:2;	/* Command packet DRQ type */
-	unsigned removable		:1;	/* Removable media */
-	unsigned device_type		:5;	/* Device type */
-	unsigned reserved13		:1;	/* Reserved */
-	unsigned protocol		:2;	/* Protocol type */
-};
-
 /* Structures related to the SELECT SENSE / MODE SENSE packet commands. */
 #define IDETAPE_BLOCK_DESCRIPTOR	0
 #define	IDETAPE_CAPABILITIES_PAGE	0x2a
@@ -3382,37 +3368,41 @@ static int idetape_chrdev_release (struct inode *inode, struct file *filp)
 }
 
 /*
- *	idetape_identify_device is called to check the contents of the
- *	ATAPI IDENTIFY command results. We return:
+ * check the contents of the ATAPI IDENTIFY command results. We return:
  *
- *	1	If the tape can be supported by us, based on the information
- *		we have so far.
+ * 1 - If the tape can be supported by us, based on the information we have so
+ * far.
  *
- *	0 	If this tape driver is not currently supported by us.
+ * 0 - If this tape driver is not currently supported by us.
  */
-static int idetape_identify_device (ide_drive_t *drive)
+static int idetape_identify_device(ide_drive_t *drive)
 {
-	struct idetape_id_gcw gcw;
-	struct hd_driveid *id = drive->id;
+
+	u8 gcw[2];
+	u8 protocol, device_type, removable, packet_size;
 
 	if (drive->id_read == 0)
 		return 1;
 
-	*((unsigned short *) ...
[ message continues ]

This function was being used only at one place so fold it in there.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   36 ++++++++++++++++--------------------
 1 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index b4944ef..ae2c76d 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -771,25 +771,6 @@ static void idetape_activate_next_stage(ide_drive_t *drive)
 }
 
 /*
- *	idetape_increase_max_pipeline_stages is a part of the feedback
- *	loop which tries to find the optimum number of stages. In the
- *	feedback loop, we are starting from a minimum maximum number of
- *	stages, and if we sense that the pipeline is empty, we try to
- *	increase it, until we reach the user compile time memory limit.
- */
-static void idetape_increase_max_pipeline_stages (ide_drive_t *drive)
-{
-	idetape_tape_t *tape = drive->driver_data;
-	int increase = (tape->max_pipeline - tape->min_pipeline) / 10;
-
-	debug_log(DBG_PROCS, "Enter %s\n", __func__);
-
-	tape->max_stages += max(increase, 1);
-	tape->max_stages = max(tape->max_stages, tape->min_pipeline);
-	tape->max_stages = min(tape->max_stages, tape->max_pipeline);
-}
-
-/*
  *	idetape_kfree_stage calls kfree to completely free a stage, along with
  *	its related buffers.
  */
@@ -937,7 +918,22 @@ static int idetape_end_request(ide_drive_t *drive, int uptodate, int nr_sects)
 			(void)ide_do_drive_cmd(drive, tape->active_data_rq,
 						ide_end);
 		} else if (!error) {
-				idetape_increase_max_pipeline_stages(drive);
+			/*
+			 * This is a part of the feedback loop which tries to
+			 * find the optimum number of stages. We are starting
+			 * from a minimum maximum number of stages, and if we
+			 * sense that the pipeline is empty, we try to increase
+			 * it, until we reach the user compile time memory
+			 * limit.
+			 */
+			int i = (tape->max_pipeline - tape->min_pipeline) / 10;
+
+			tape->max_stages ...
[ message continues ]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 24f048f..cfcf5b0 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -3681,9 +3681,6 @@ failed:
 	return -ENODEV;
 }
 
-MODULE_DESCRIPTION("ATAPI Streaming TAPE Driver");
-MODULE_LICENSE("GPL");
-
 static void __exit idetape_exit (void)
 {
 	driver_unregister(&idetape_driver.gen_driver);
@@ -3726,3 +3723,5 @@ MODULE_ALIAS("ide:*m-tape*");
 module_init(idetape_init);
 module_exit(idetape_exit);
 MODULE_ALIAS_CHARDEV_MAJOR(IDETAPE_MAJOR);
+MODULE_DESCRIPTION("ATAPI Streaming TAPE Driver");
+MODULE_LICENSE("GPL");
-- 
1.5.3.7

--

[ message continues ]

... by adding a new typedef function pointer idetape_io_buf in order to call
the proper buffer i/o handler depending on the data direction.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   55 +++++++++++++++++++++++++----------------------
 1 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index b15dd17..2857965 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1105,18 +1105,22 @@ static void idetape_postpone_request (ide_drive_t *drive)
 }
 
 /*
- *	idetape_pc_intr is the usual interrupt handler which will be called
- *	during a packet command. We will transfer some of the data (as
- *	requested by the drive) and will re-point interrupt handler to us.
- *	When data transfer is finished, we will act according to the
- *	algorithm described before idetape_issue_packet_command.
- *
+ * This is the usual interrupt handler which will be called during a packet
+ * command. We will transfer some of the data (as requested by the drive) and
+ * will re-point interrupt handler to us. When data transfer is finished, we
+ * will act according to the algorithm described before
+ * idetape_issue_packet_command.
  */
-static ide_startstop_t idetape_pc_intr (ide_drive_t *drive)
+
+typedef void idetape_io_buf(ide_drive_t *, idetape_pc_t *, unsigned int);
+
+static ide_startstop_t idetape_pc_intr(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
 	idetape_tape_t *tape = drive->driver_data;
 	idetape_pc_t *pc = tape->pc;
+	xfer_func_t *xferfunc;
+	idetape_io_buf *iobuf;
 	unsigned int temp;
 #if SIMULATE_ERRORS
 	static int error_sim_count = 0;
@@ -1184,7 +1188,8 @@ static ide_startstop_t idetape_pc_intr (ide_drive_t *drive)
 			debug_log(DBG_ERR, "%s: I/O error\n", tape->name);
 
 			if (pc->c[0] == REQUEST_SENSE) {
-				printk(KERN_ERR "ide-tape: I/O error in request sense command\n");
+				printk(KERN_ERR "ide-tape: I/O error in request"
+						" sense ...
[ message continues ]

Also remove flag IDETAPE_READ_ERROR since it is unused.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |  226 +++++++++++++++++++++++++----------------------
 1 files changed, 120 insertions(+), 106 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 4fee160..1c4f94c 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -207,24 +207,24 @@ typedef struct idetape_packet_command_s {
 	u8 *current_position;			/* Pointer into the above buffer */
 	ide_startstop_t (*callback) (ide_drive_t *);	/* Called when this packet command is completed */
 	u8 pc_buffer[IDETAPE_PC_BUFFER_SIZE];	/* Temporary buffer */
-	unsigned long flags;			/* Status/Action bit flags: long for set_bit */
+	unsigned int flags;
 } idetape_pc_t;
 
-/*
- *	Packet command flag bits.
- */
-/* Set when an error is considered normal - We won't retry */
-#define	PC_ABORT			0
-/* 1 When polling for DSC on a media access command */
-#define PC_WAIT_FOR_DSC			1
-/* 1 when we prefer to use DMA if possible */
-#define PC_DMA_RECOMMENDED		2
-/* 1 while DMA in progress */
-#define	PC_DMA_IN_PROGRESS		3
-/* 1 when encountered problem during DMA */
-#define	PC_DMA_ERROR			4
-/* Data direction */
-#define	PC_WRITING			5
+/* Packet command flag bits. */
+enum {
+	/* Set when an error is considered normal - We won't retry */
+	PC_FL_ABORT		= (1 << 0),
+	/* 1 When polling for DSC on a media access command */
+	PC_FL_WAIT_FOR_DSC	= (1 << 1),
+	/* 1 when we prefer to use DMA if possible */
+	PC_FL_DMA_RECOMMENDED	= (1 << 2),
+	/* 1 while DMA in progress */
+	PC_FL_DMA_IN_PROGRESS	= (1 << 3),
+	/* 1 when encountered problem during DMA */
+	PC_FL_DMA_ERROR		= (1 << 4),
+	/* Data direction */
+	PC_FL_WRITING		= (1 <<	5),
+};
 
 /*
  *	A pipeline stage.
@@ -354,8 +354,7 @@ typedef struct ide_tape_obj {
 	/* Wasted space in each stage */
 	int excess_bh_size;
 
-	/* Status/Action flags: long for set_bit */
-	unsigned long flags;
+	unsigned ...
[ message continues ]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   32 +++++++++++++++-----------------
 1 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index ae2c76d..4fee160 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1053,7 +1053,7 @@ static void idetape_postpone_request (ide_drive_t *drive)
  * command. We will transfer some of the data (as requested by the drive) and
  * will re-point interrupt handler to us. When data transfer is finished, we
  * will act according to the algorithm described before
- * idetape_issue_packet_command.
+ * idetape_issue_pc.
  */
 
 typedef void idetape_io_buf(ide_drive_t *, idetape_pc_t *, unsigned int);
@@ -1233,7 +1233,7 @@ static ide_startstop_t idetape_pc_intr(ide_drive_t *drive)
  *
  *	The handling will be done in three stages:
  *
- *	1.	idetape_issue_packet_command will send the packet command to the
+ *	1.	idetape_issue_pc will send the packet command to the
  *		drive, and will set the interrupt handler to idetape_pc_intr.
  *
  *	2.	On each interrupt, idetape_pc_intr will be called. This step
@@ -1308,7 +1308,7 @@ static ide_startstop_t idetape_transfer_pc(ide_drive_t *drive)
 	return ide_started;
 }
 
-static ide_startstop_t idetape_issue_packet_command (ide_drive_t *drive, idetape_pc_t *pc)
+static ide_startstop_t idetape_issue_pc(ide_drive_t *drive, idetape_pc_t *pc)
 {
 	ide_hwif_t *hwif = drive->hwif;
 	idetape_tape_t *tape = drive->driver_data;
@@ -1614,7 +1614,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 	 */
 	if (tape->failed_pc != NULL &&
 	    tape->pc->c[0] == REQUEST_SENSE) {
-		return idetape_issue_packet_command(drive, tape->failed_pc);
+		return idetape_issue_pc(drive, tape->failed_pc);
 	}
 	if (postponed_rq != NULL)
 		if (rq != postponed_rq) {
@@ -1707,7 +1707,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 	}
 	BUG();
 out:
-	return ...
[ message continues ]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 342ec50..24f048f 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -39,9 +39,9 @@
 #include <scsi/scsi.h>
 
 #include <asm/byteorder.h>
-#include <asm/irq.h>
-#include <asm/uaccess.h>
-#include <asm/io.h>
+#include <linux/irq.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
 #include <asm/unaligned.h>
 #include <linux/mtio.h>
 
-- 
1.5.3.7

--

[ message continues ]

Also, remove redundant ones and cleanup whitespace.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |  725 +++++++++++++++++++----------------------------
 1 files changed, 293 insertions(+), 432 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 175d507..a80f8d9 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -73,37 +73,34 @@ enum {
 
 
 /*
- *	Pipelined mode parameters.
+ * Pipelined mode parameters.
  *
- *	We try to use the minimum number of stages which is enough to
- *	keep the tape constantly streaming. To accomplish that, we implement
- *	a feedback loop around the maximum number of stages:
+ * We try to use the minimum number of stages which is enough to keep the tape
+ * constantly streaming. To accomplish that, we implement a feedback loop around
+ * the maximum number of stages:
  *
- *	We start from MIN maximum stages (we will not even use MIN stages
- *      if we don't need them), increment it by RATE*(MAX-MIN)
- *	whenever we sense that the pipeline is empty, until we reach
- *	the optimum value or until we reach MAX.
+ * We start from MIN maximum stages (we will not even use MIN stages if we don't
+ * need them), increment it by RATE*(MAX-MIN) whenever we sense that the
+ * pipeline is empty, until we reach the optimum value or until we reach MAX.
  *
- *	Setting the following parameter to 0 is illegal: the pipelined mode
- *	cannot be disabled (idetape_calculate_speeds() divides by
- *	tape->max_stages.)
+ * Setting the following parameter to 0 is illegal: the pipelined mode cannot be
+ * disabled (idetape_calculate_speeds() divides by tape->max_stages.)
  */
 #define IDETAPE_MIN_PIPELINE_STAGES	  1
 #define IDETAPE_MAX_PIPELINE_STAGES	400
 #define IDETAPE_INCREASE_STAGES_RATE	 20
 
 /*
- *	After each failed packet command we issue a request sense command
- *	and retry the packet command IDETAPE_MAX_PC_RETRIES times.
+ * After each failed packet command we issue a ...
[ message continues ]

the above paragraph is true only for point 2.



I dropped the above chunks (they don't seem to add any value)


replaced tabs with spaces


this change was also dropped


same here


--

[ message continues ]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 5c88885..2e6198f 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -15,7 +15,7 @@
  * Documentation/ide/ChangeLog.ide-tape.1995-2002
  */
 
-#define IDETAPE_VERSION "1.19"
+#define IDETAPE_VERSION "1.20"
 
 #include <linux/module.h>
 #include <linux/types.h>
-- 
1.5.3.7

--

[ message continues ]

... thus decreasing checkpatch.pl errors to 0.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |  801 ++++++++++++++++++++++++++++--------------------
 1 files changed, 468 insertions(+), 333 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 7e998c4..5c88885 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -486,7 +486,7 @@ enum {
  * The variables below are used for the character device interface. Additional
  * state variables are defined in our ide_drive_t structure.
  */
-static struct ide_tape_obj * idetape_devs[MAX_HWIFS * MAX_DRIVES];
+static struct ide_tape_obj *idetape_devs[MAX_HWIFS * MAX_DRIVES];
 
 #define ide_tape_f(file) ((file)->private_data)
 
@@ -502,20 +502,21 @@ static struct ide_tape_obj *ide_tape_chrdev_get(unsigned int i)
 	return tape;
 }
 
-static int idetape_chrdev_release (struct inode *inode, struct file *filp);
-static void idetape_write_release (ide_drive_t *drive, unsigned int minor);
+static int idetape_chrdev_release(struct inode *inode, struct file *filp);
+static void idetape_write_release(ide_drive_t *drive, unsigned int minor);
 
 /*
  * Too bad. The drive wants to send us data which we are not ready to accept.
  * Just throw it away.
  */
-static void idetape_discard_data (ide_drive_t *drive, unsigned int bcount)
+static void idetape_discard_data(ide_drive_t *drive, unsigned int bcount)
 {
 	while (bcount--)
 		(void) HWIF(drive)->INB(IDE_DATA_REG);
 }
 
-static void idetape_input_buffers (ide_drive_t *drive, idetape_pc_t *pc, unsigned int bcount)
+static void idetape_input_buffers(ide_drive_t *drive, idetape_pc_t *pc,
+		unsigned int bcount)
 {
 	struct idetape_bh *bh = pc->bh;
 	int count;
@@ -527,8 +528,11 @@ static void idetape_input_buffers (ide_drive_t *drive, idetape_pc_t *pc, unsigne
 			idetape_discard_data(drive, bcount);
 			return;
 		}
-		count = min((unsigned int)(bh->b_size - atomic_read(&bh->b_count)), ...
[ message continues ]

I did a couple of changes while merging the patch...
(I hope that I didn't break something in the process :)

From: Borislav Petkov <petkovbb@googlemail.com>
Subject: [PATCH 20/22] ide-tape: cleanup the remaining codestyle issues

... thus decreasing checkpatch.pl errors to 0.

Bart:
- remove needless function prototypes while at it
- remove needless parentheses while at it
- add missing KERN_ level to ide_tape_probe()
- other minor fixups

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-tape.c |  830 ++++++++++++++++++++++++++++---------------------
 1 file changed, 479 insertions(+), 351 deletions(-)

Index: b/drivers/ide/ide-tape.c
===================================================================
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -506,7 +506,7 @@ enum {
  * The variables below are used for the character device interface. Additional
  * state variables are defined in our ide_drive_t structure.
  */
-static struct ide_tape_obj * idetape_devs[MAX_HWIFS * MAX_DRIVES];
+static struct ide_tape_obj *idetape_devs[MAX_HWIFS * MAX_DRIVES];
 
 #define ide_tape_f(file) ((file)->private_data)
 
@@ -522,20 +522,18 @@ static struct ide_tape_obj *ide_tape_chr
 	return tape;
 }
 
-static int idetape_chrdev_release (struct inode *inode, struct file *filp);
-static void idetape_write_release (ide_drive_t *drive, unsigned int minor);
-
 /*
  * Too bad. The drive wants to send us data which we are not ready to accept.
  * Just throw it away.
  */
-static void idetape_discard_data (ide_drive_t *drive, unsigned int bcount)
+static void idetape_discard_data(ide_drive_t *drive, unsigned int bcount)
 {
 	while (bcount--)
 		(void) HWIF(drive)->INB(IDE_DATA_REG);
 }
 
-static void idetape_input_buffers (ide_drive_t *drive, idetape_pc_t *pc, unsigned int bcount)
+static void idetape_input_buffers(ide_drive_t *drive, idetape_pc_t *pc,
+				  unsigned int bcount)
 ...
[ message continues ]

- last_frame_position: only being written to once
- firmware_revision, product_id, vendor_id: used once, remove from struct
  idetape_tape_t and deal with them locally
- firmware_revision_num: only written to once
- tape_still_time_begin: completely unused
- tape_still_time: never written to; remove corresponding code chunk
- uncontrolled_last_pipeline_head: only once written to
- blocks_in_buffer: only written to

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   45 +++++++++++----------------------------------
 1 files changed, 11 insertions(+), 34 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index e0e8184..126e8a9 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -311,8 +311,6 @@ typedef struct ide_tape_obj {
 	u8 partition;
 	/* Current block */
 	unsigned int first_frame_position;
-	unsigned int last_frame_position;
-	unsigned int blocks_in_buffer;
 
 	/*
 	 *	Last error information
@@ -399,11 +397,6 @@ typedef struct ide_tape_obj {
 	int avg_size;
 	int avg_speed;
 
-	char vendor_id[10];
-	char product_id[18];
-	char firmware_revision[6];
-	int firmware_revision_num;
-
 	/* the door is currently locked */
 	int door_locked;
 	/* the tape hardware is write protected */
@@ -441,12 +434,6 @@ typedef struct ide_tape_obj {
 	int measure_insert_time;
 
 	/*
-	 * Measure tape still time, in milliseconds
-	 */
-	unsigned long tape_still_time_begin;
-	int tape_still_time;
-
-	/*
 	 * Speed regulation negative feedback loop
 	 */
 	int speed_control;
@@ -454,7 +441,6 @@ typedef struct ide_tape_obj {
 	int controlled_pipeline_head_speed;
 	int uncontrolled_pipeline_head_speed;
 	int controlled_last_pipeline_head;
-	int uncontrolled_last_pipeline_head;
 	unsigned long uncontrolled_pipeline_head_time;
 	unsigned long controlled_pipeline_head_time;
 	int controlled_previous_pipeline_head;
@@ -1696,8 +1682,6 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 ...
[ message continues ]

Hello.




    It was wrong to call ide_fixstring() on unterminated strings and expecting 
them to become terminated strings after that; plus it was useless to add 2 
characters padding at the end. When these variables were the fields of 'struct 
ide_tape_obj', those bytes were 0 because of the variable of this type being a 
static array. When they became local variables, they got garbage bytes at the 
end which ide_fixdriveid() either honestly copied when compressing spaces or 

    Should've rather changed the string format to print only N characters max...

MBR, Sergei
--

[ message continues ]

On Mon, Sep 22, 2008 at 3:25 PM, Sergei Shtylyov




-- 
Regards/Gruss,
Boris
--

[ message continues ]

The fix that I have suggested isn't at all similar.  We don't need to 
waste memory on extra bytes, and even less on them being of the 'static' 
memory class...

MBR, Sergei
--

[ message continues ]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index cfcf5b0..4a12c0b 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -3629,10 +3629,6 @@ static int ide_tape_probe(ide_drive_t *drive)
 		printk("ide-tape: passing drive %s to ide-scsi emulation.\n", drive->name);
 		goto failed;
 	}
-	if (strstr(drive->id->model, "OnStream DI-")) {
-		printk(KERN_WARNING "ide-tape: Use drive %s with ide-scsi emulation and osst.\n", drive->name);
-		printk(KERN_WARNING "ide-tape: OnStream support will be removed soon from ide-tape!\n");
-	}
 	tape = kzalloc(sizeof (idetape_tape_t), GFP_KERNEL);
 	if (tape == NULL) {
 		printk(KERN_ERR "ide-tape: %s: Can't allocate a tape structure\n", drive->name);
-- 
1.5.3.7

--

[ message continues ]

Shorten some member names not too aggressively since this driver might be gone
anyway soon.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |  210 ++++++++++++++++++++++++++----------------------
 1 files changed, 113 insertions(+), 97 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 126e8a9..0b5ccce 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -299,10 +299,8 @@ typedef struct ide_tape_obj {
 	/* Timer used to poll for dsc */
 	struct timer_list dsc_timer;
 	/* Read/Write dsc polling frequency */
-	unsigned long best_dsc_rw_frequency;
-	/* The current polling frequency */
-	unsigned long dsc_polling_frequency;
-	/* Maximum waiting time */
+	unsigned long best_dsc_rw_freq;
+	unsigned long dsc_poll_freq;
 	unsigned long dsc_timeout;
 
 	/*
@@ -310,7 +308,7 @@ typedef struct ide_tape_obj {
 	 */
 	u8 partition;
 	/* Current block */
-	unsigned int first_frame_position;
+	unsigned int first_frame;
 
 	/*
 	 *	Last error information
@@ -326,11 +324,8 @@ typedef struct ide_tape_obj {
 	/* Current character device data transfer direction */
 	u8 chrdev_dir;
 
-	/*
-	 *	Device information
-	 */
-	/* Usually 512 or 1024 bytes */
-	unsigned short tape_block_size;
+	/* tape block size, usu. 512 or 1024 bytes */
+	unsigned short blk_size;
 	int user_bs_factor;
 
 	/* Copy of the tape's Capabilities and Mechanical Page */
@@ -349,8 +344,8 @@ typedef struct ide_tape_obj {
 	 *	The data buffer size is chosen based on the tape's
 	 *	recommendation.
 	 */
-	/* Pointer to the request which is waiting in the device request queue */
-	struct request *active_data_request;
+	/* Ptr to the request which is waiting in the device request queue */
+	struct request *active_data_rq;
 	/* Data buffer size (chosen based on the tape's recommendation */
 	int stage_size;
 	idetape_stage_t *merge_stage;
@@ -388,7 +383,7 @@ typedef struct ide_tape_obj {
 	/* Status/Action flags: long for set_bit ...
[ message continues ]

for some reason gcc doesn't seem to optimize the new code as well as
the old one (=> driver size goes up instead of staying unchanged)

interdiff between original patch and merged version:

diff -u b/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
--- b/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -324,7 +324,7 @@
 	/* Current character device data transfer direction */
 	u8 chrdev_dir;
 
-	/* tape block size, usu. 512 or 1024 bytes */
+	/* tape block size, usually 512 or 1024 bytes */
 	unsigned short blk_size;
 	int user_bs_factor;
 
@@ -1580,8 +1580,8 @@
 	pc->bh = bh;
 	atomic_set(&bh->b_count, 0);
 	pc->buffer = NULL;
-	pc->buffer_size		= length * tape->blk_size;
-	pc->request_transfer	= length * tape->blk_size;
+	pc->buffer_size = length * tape->blk_size;
+	pc->request_transfer = pc->buffer_size;
 	if (pc->request_transfer == tape->stage_size)
 		set_bit(PC_DMA_RECOMMENDED, &pc->flags);
 }
@@ -1619,8 +1619,8 @@
 	pc->b_data = bh->b_data;
 	pc->b_count = atomic_read(&bh->b_count);
 	pc->buffer = NULL;
-	pc->request_transfer	= length * tape->blk_size;
-	pc->buffer_size		= length * tape->blk_size;
+	pc->buffer_size = length * tape->blk_size;
+	pc->request_transfer = pc->buffer_size;
 	if (pc->request_transfer == tape->stage_size)
 		set_bit(PC_DMA_RECOMMENDED, &pc->flags);
 }
--

[ message continues ]

Yeah, i did that only because checkpatch.pl complained that multiple assignments
should be avoided. Now it looks kinda dumb that way besides improving readability
so converting it to the best form w.r.t generating smaller binary would be a reason
good enough to ignore checkpatch.pl in that case.

-- 
Regards/Gruß,
    Boris.
--

[ message continues ]

Spotted by Sergei Shtylyov.

CC: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 4a12c0b..7e998c4 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -3273,8 +3273,8 @@ static int idetape_identify_device(ide_drive_t *drive)
 	else if (!removable)
 		printk(KERN_ERR "ide-tape: The removable flag is not set\n");
 	else if (packet_size != 0) {
-		printk(KERN_ERR "ide-tape: Packet size (0x%02x) is not 12 "
-				"bytes long\n", packet_size);
+		printk(KERN_ERR "ide-tape: Packet size (0x%02x) is not 12"
+				" bytes\n", packet_size);
 	} else
 		return 1;
 	return 0;
-- 
1.5.3.7

--

[ message continues ]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 Documentation/feature-removal-schedule.txt |   14 ++++++++++++--
 drivers/ide/ide-tape.c                     |    5 +++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index a7d9d17..147bb63 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -264,8 +264,6 @@ Who:	Thomas Gleixner <tglx@linutronix.de>
 
 ---------------------------
 
----------------------------
-
 What:	i2c-i810, i2c-prosavage and i2c-savage4
 When:	May 2008
 Why:	These drivers are superseded by i810fb, intelfb and savagefb.
@@ -338,3 +336,15 @@ Why:	The support code for the old firmware hurts code readability/maintainabilit
 	and slightly hurts runtime performance. Bugfixes for the old firmware
 	are not provided by Broadcom anymore.
 Who:	Michael Buesch <mb@bu3sch.de>
+
+---------------------------
+
+What:	ide-tape driver
+When:	July 2008
+Files:	drivers/ide/ide-tape.c
+Why:	This driver might not have any users anymore and maintaining it for no
+	reason is an effort no one wants to make.
+Who:	Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>, Borislav Petkov
+	<petkovbb@googlemail.com>
+
+---------------------------
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 2e6198f..86f206b 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -3803,6 +3803,11 @@ static int ide_tape_probe(ide_drive_t *drive)
 	g->fops = &idetape_block_ops;
 	ide_register_region(g);
 
+	printk(KERN_WARNING "It is possible that this driver does not have any"
+		" users anymore and, as a result, it will be REMOVED soon."
+		" Please notify Bart <bzolnier@gmail.com> or Boris"
+		" <petkovbb@gmail.com> in case you still need it.\n");
+
 	return 0;
 
 out_free_tape:
-- 
1.5.3.7

--

[ message continues ]

Hi Borislav,



applied #1-7, #9-10, #13-22 (+queued all of them for 2.6.25)

w.r.t. #8 I'm waiting for Jens to comment on blk_{get,put}_request() approach

w.r.t. #11 ide-tape uses char devices and supports DSC so it is not as obvious
as in ide-floppy case that all atomic bitops can be just removed (extra audit
and some time -mm are required) so please resync/resubmit

#12 is already in Linus' tree

Bart
--

[ message continues ]

On Tue, Feb 05, 2008 at 02:20:22AM +0100, Bartlomiej Zolnierkiewicz wrote:


Ok, here's what i think we should do here: There are two flags that handle DSC:
PC_FL_WAIT_FOR_DSC and IDETAPE_FL_IGNORE_DSC. The first one is per pc and is set in
all the packet command init functions ..create_bla_cmd() after their callers have
created a pc on the stack and reached its ptr down for initialization. This case
is carefree since the bit will be tested first in the interrupt handler and this
happens only after the pc is queued (ide_do_drive_cmd()) into the request buffer.

The other flag, IDETAPE_FL_IGNORE_DSC, is polled for in the request handler and
can be set when a pc is being retried and we should leave only those atomic
tests intact, imho, but i'm definitely gonna need a second opinion here.

---

commit 1ed8ae92249d5dff7af4ee88710ea08ff3f3356f
Author: Borislav Petkov <petkovbb@gmail.com>
Date:   Tue Feb 5 08:05:35 2008 +0100

    ide-tape: remove atomic test/set macros
    
    Also, since the driver supports DSC, leave the atomic tests
    for the IDETAPE_FL_IGNORE_DSC bit untouched because this is polled
    for in the request handler and can be set in the interrupt
    handler through idetape_retry_pc() after enabling interrupts.
    
    Finally, remove flag IDETAPE_READ_ERROR since it is unused.
    
    Signed-off-by: Borislav Petkov <petkovbb@gmail.com>

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index e59e49e..9455ce4 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -206,24 +206,24 @@ typedef struct idetape_packet_command_s {
 	/* Temporary buffer */
 	u8 pc_buffer[IDETAPE_PC_BUFFER_SIZE];
 	/* Status/Action bit flags: long for set_bit */
-	unsigned long flags;
+	unsigned int flags;
 } idetape_pc_t;
 
-/*
- *	Packet command flag bits.
- */
-/* Set when an error is considered normal - We won't retry */
-#define	PC_ABORT			0
-/* 1 When polling for DSC on a media access command */
-#define PC_WAIT_FOR_DSC			1
-/* 1 when we ...
[ message continues ]

... and while we're at it ...

commit c824f79fe4040f7541d7e35c546bb57a22d2fe11
Author: Borislav Petkov <petkovbb@gmail.com>
Date:   Wed Feb 6 06:23:10 2008 +0100

    ide-tape: move all struct and other defs to the top
    
    Signed-off-by: Borislav Petkov <petkovbb@gmail.com>

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 9455ce4..398aea8 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -225,6 +225,69 @@ enum {
 	PC_FL_WRITING		= (1 <<	5),
 };
 
+/* Tape door status */
+#define DOOR_UNLOCKED			0
+#define DOOR_LOCKED			1
+#define DOOR_EXPLICITLY_LOCKED		2
+
+/* Tape flag bits values. */
+enum {
+	IDETAPE_FL_IGNORE_DSC		= (1 << 0),
+	/* 0 When the tape position is unknown */
+	IDETAPE_FL_ADDRESS_VALID	= (1 <<	1),
+	/* Device already opened */
+	IDETAPE_FL_BUSY			= (1 << 2),
+	/* Error detected in a pipeline stage */
+	IDETAPE_FL_PIPELINE_ERR	= (1 <<	3),
+	/* Attempt to auto-detect the current user block size */
+	IDETAPE_FL_DETECT_BS		= (1 << 4),
+	/* Currently on a filemark */
+	IDETAPE_FL_FILEMARK		= (1 << 5),
+	/* DRQ interrupt device */
+	IDETAPE_FL_DRQ_INTERRUPT	= (1 << 6),
+	/* pipeline active */
+	IDETAPE_FL_PIPELINE_ACTIVE	= (1 << 7),
+	/* 0 = no tape is loaded, so we don't rewind after ejecting */
+	IDETAPE_FL_MEDIUM_PRESENT	= (1 << 8),
+};
+
+/* A define for the READ BUFFER command */
+#define IDETAPE_RETRIEVE_FAULTY_BLOCK	6
+
+/* Some defines for the SPACE command */
+#define IDETAPE_SPACE_OVER_FILEMARK	1
+#define IDETAPE_SPACE_TO_EOD		3
+
+/* Some defines for the LOAD UNLOAD command */
+#define IDETAPE_LU_LOAD_MASK		1
+#define IDETAPE_LU_RETENSION_MASK	2
+#define IDETAPE_LU_EOT_MASK		4
+
+/*
+ * Special requests for our block device strategy routine.
+ *
+ * In order to service a character device command, we add special requests to
+ * the tail of our block device request queue and wait for their completion.
+ */
+
+enum {
+	REQ_IDETAPE_PC1		= (1 << 0), /* packet command (first stage) ...
[ message continues ]

looks good, but please do it before
"remove atomic test/set macros" patch
--

[ message continues ]

Hi Bart,

thanks for the update earlier. I'll look into blk_{get,put}_request later and
try it on idefloppy. In the meantime, here are the patches as requested:

---
From 0d91862fc802d6f5aa79947b2685de6c209236f2 Mon Sep 17 00:00:00 2001
From: Borislav Petkov <petkovbb@gmail.com>
Date: Sat, 9 Feb 2008 19:48:13 +0100
Subject: [PATCH 1/2] ide-tape: move all struct and other defs at the top

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |  107 ++++++++++++++++++++++++-----------------------
 1 files changed, 55 insertions(+), 52 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index b3269d7..1fff560 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -225,6 +225,61 @@ typedef struct idetape_packet_command_s {
 /* Data direction */
 #define	PC_WRITING			5
 
+/* Tape door status */
+#define DOOR_UNLOCKED			0
+#define DOOR_LOCKED			1
+#define DOOR_EXPLICITLY_LOCKED		2
+
+/*
+ *	Tape flag bits values.
+ */
+#define IDETAPE_IGNORE_DSC		0
+/* 0 When the tape position is unknown */
+#define IDETAPE_ADDRESS_VALID		1
+#define IDETAPE_BUSY			2	/* Device already opened */
+/* Error detected in a pipeline stage */
+#define IDETAPE_PIPELINE_ERROR		3
+/* Attempt to auto-detect the current user block size */
+#define IDETAPE_DETECT_BS		4
+#define IDETAPE_FILEMARK		5	/* Currently on a filemark */
+#define IDETAPE_DRQ_INTERRUPT		6	/* DRQ interrupt device */
+#define IDETAPE_READ_ERROR		7
+#define IDETAPE_PIPELINE_ACTIVE		8	/* pipeline active */
+/* 0 = no tape is loaded, so we don't rewind after ejecting */
+#define IDETAPE_MEDIUM_PRESENT		9
+
+/* Some defines for the SPACE command */
+#define IDETAPE_SPACE_OVER_FILEMARK	1
+#define IDETAPE_SPACE_TO_EOD		3
+
+/* Some defines for the LOAD UNLOAD command */
+#define IDETAPE_LU_LOAD_MASK		1
+#define IDETAPE_LU_RETENSION_MASK	2
+#define IDETAPE_LU_EOT_MASK		4
+
+/*
+ * Special requests for our block device strategy routine.
+ *
+ * In order to service ...
[ message continues ]

since there is no need to move defines for tape flags around (as they are
completely rewritten by the next patch) I removed this change from this patch
making it a bit simpler/smaller:

 drivers/ide/ide-tape.c |   74 ++++++++++++++++++++++++-------------------------
 1 file changed, 37 insertions(+), 37 deletions(-)
--

[ message continues ]

Hi,


Actually for this flag it looks safe because the new request shouldn't be
queued while the old one is still active (IOW there should be no simultaneous
idetape_do_request() and idetape_end_request() calls).  OTOH some other
flags (especially IDETAPE_PIPELINE_ACTIVE) looks un-safe (pileline can be
active while ->open on character device happens).

Given that this driver is currently scheduled for removal and that complete
audit/analysis would be quite costly I don't think that removing atomic bitops
from tape->flags is worth it (for pc->flags it is fine).

Could you please recast the patch accordingly?




please leave it as 'unsigned long'

[ besides I don't think that mixing atomic and non-atomic access on

Why not IDETAPE_FLAG_*?

The rest of the patch looks good.
--

[ message continues ]

From 92dd5c1cfb27c0945894a3a055098290047d1ff0 Mon Sep 17 00:00:00 2001
From: Borislav Petkov <petkovbb@gmail.com>
Date: Sat, 9 Feb 2008 20:33:48 +0100
Subject: [PATCH 2/2] ide-tape: remove atomic test/set macros for packet commands

Removing the atomic tests for pc's is unobjectionable. Since this driver will
probably go to /dev/null soon, the atomic tests for tape->flags are left in
place for there are some situations where they're needed (chrdev DSC handling,
low level pipeline operation and so on). While at it, rename all test/set flag
bit defines explicitly to *_FLAG_* for clarity.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |  208 +++++++++++++++++++++++++-----------------------
 1 files changed, 109 insertions(+), 99 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 1fff560..09ff9b0 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -209,44 +209,47 @@ typedef struct idetape_packet_command_s {
 	unsigned long flags;
 } idetape_pc_t;
 
-/*
- *	Packet command flag bits.
- */
-/* Set when an error is considered normal - We won't retry */
-#define	PC_ABORT			0
-/* 1 When polling for DSC on a media access command */
-#define PC_WAIT_FOR_DSC			1
-/* 1 when we prefer to use DMA if possible */
-#define PC_DMA_RECOMMENDED		2
-/* 1 while DMA in progress */
-#define	PC_DMA_IN_PROGRESS		3
-/* 1 when encountered problem during DMA */
-#define	PC_DMA_ERROR			4
-/* Data direction */
-#define	PC_WRITING			5
+/* Packet command flag bits. */
+enum {
+	/* Set when an error is considered normal - We won't retry */
+	PC_FLAG_ABORT		= (1 << 0),
+	/* 1 When polling for DSC on a media access command */
+	PC_FLAG_WAIT_FOR_DSC	= (1 << 1),
+	/* 1 when we prefer to use DMA if possible */
+	PC_FLAG_DMA_RECOMMENDED	= (1 << 2),
+	/* 1 while DMA in progress */
+	PC_FLAG_DMA_IN_PROGRESS	= (1 << 3),
+	/* 1 when encountered problem during DMA */
+	PC_FLAG_DMA_ERROR		= (1 << 4),
+	/* Data direction ...
[ message continues ]

applied
--

[ message continues ]